Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Segfault - strcpy() copying into array

Reply
Thread Tools

Segfault - strcpy() copying into array

 
 
Seebs
Guest
Posts: n/a
 
      01-11-2011
On 2011-01-10, arnuld <(E-Mail Removed)> wrote:
>> On Mon, 10 Jan 2011 08:52:56 +0000, Seebs wrote:
>> I am starting to wonder whether you should just give up on C.


> I won't


Well.

> gcc -ansi -pedantic -Wall -Wextra does not give any warning:


You need at least -O1 for uninitialized variable detections.

> I think I missed out the bug probably because my brain was engaged
> somewhere else, I am trying to solve an issue of Segmentation Fault in
> some code (which I can't post as its proprietary). There is a linked list
> of such kind of struct pointers and a for loop is going on it. NULL != st-
>>title passes through but '\0' == st->title[0] gives a Segfault in some

> very rare situations. Don't know why its happening but unlike this 2 line
> example that code does have a malloc() with a null check. My brain was
> totally into that when I wrote this example.


In that case, the chances are that st->title is an invalid pointer which
isn't null. Say, a pointer to memory that's already been freed, or otherwise
to an object which no longer exists. Another common source would be that
the pointer object itself got overwritten such that it's no longer a
meaningful pointer.

Seriously, though, if you can look at dereferencing of a variable which
was just declared without any kind of initialization, and not see a problem
even when trying to think about the problem, you are not ready to be doing
this stuff, and you shouldn't be dealing with "proprietary" code 'cuz you're
not ready to do this professionally yet.

What ever happened to professional ethics?

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / http://www.velocityreviews.com/forums/(E-Mail Removed)
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
I am not speaking for my employer, although they do rent some of my opinions.
 
Reply With Quote
 
 
 
 
arnuld
Guest
Posts: n/a
 
      01-11-2011
> On Tue, 11 Jan 2011 00:11:24 +0000, Seebs wrote:

> ...SNIP...


> In that case, the chances are that st->title is an invalid pointer which
> isn't null. Say, a pointer to memory that's already been freed, or
> otherwise to an object which no longer exists. Another common source
> would be that the pointer object itself got overwritten such that it's
> no longer a meaningful pointer.


I solved the problem before I read your post and reason is *exactly* what
you stated. Pointer was getting overwritten. How ? , well, function-1
calls function-2 which calls functions-3 and which in turn calls
function-4 and function-4 has a malloc() which was never free()ed, so it
was continually allocating memory. I guess at some point it was
overwriting other pointers.



> Seriously, though, if you can look at dereferencing of a variable which
> was just declared without any kind of initialization, and not see a
> problem even when trying to think about the problem, you are not ready
> to be doing this stuff, and you shouldn't be dealing with "proprietary"
> code 'cuz you're not ready to do this professionally yet.


If I should not do the work then I think majority of programmers in
Indian corporate should resign because I have programmers who earn 2 to 8
times more than me and they always trust gdb more than putting error
checks in program. I have yet to meet a programmers who checks the return
value of strtoul(), they says its a waste to do such things, business
reasons are more important e.g. deadline and developing code faster. I
agree that I am quite incompetent as compared to programmers at clc and I
am happy for that because I get to learn from people like you



> What ever happened to professional ethics?


You are too kind






--
http://www.lispmachine.wordpress.com
 
Reply With Quote
 
 
 
 
arnuld
Guest
Posts: n/a
 
      01-11-2011
> On Tue, 11 Jan 2011 04:35:39 +0000, arnuld wrote:

> I solved the problem before I read your post and reason is *exactly*
> what you stated. Pointer was getting overwritten. How ? , well,
> function-1 calls function-2 which calls functions-3 and which in turn
> calls function-4 and function-4 has a malloc() which was never free()ed,
> so it was continually allocating memory. I guess at some point it was
> overwriting other pointers.


Even after adding free(), the Segfault is still there. I am working on
it.




--
http://www.lispmachine.wordpress.com
 
Reply With Quote
 
luser- -droog
Guest
Posts: n/a
 
      01-11-2011
On Jan 10, 11:53*pm, arnuld <(E-Mail Removed)> wrote:
> > On Tue, 11 Jan 2011 04:35:39 +0000, arnuld wrote:
> > I solved the problem before I read your post and reason is *exactly*
> > what you stated. Pointer was getting overwritten. How ? , well,
> > function-1 calls function-2 which calls functions-3 and which in turn
> > calls function-4 and function-4 has a malloc() which was never free()ed,
> > so it was continually allocating memory.


That makes no sense. malloc does not continually allocate memory.
Unless function-4 is doing a fork and calling malloc in a loop.
If your malloc'ed memory is getting overwritten, it's more
likely that the previous pointer returned by malloc is being
used to write more data than the allocated size.

> I guess at some point it was
> > overwriting other pointers.

>
> Even after adding free(), the Segfault is still there. I am working on
> it.


Perhaps you need a more introductory textbook. You can't
reason properly about these things until you learn what
the words mean. Forgetting to free memory is wasteful
but not necessarily harmful. If you suspect data is being
overwritten, you should look for bad writes.
 
Reply With Quote
 
arnuld
Guest
Posts: n/a
 
      01-11-2011
> On Tue, 11 Jan 2011 00:11:24 +0000, Seebs wrote:

> In that case, the chances are that st->title is an invalid pointer which
> isn't null. Say, a pointer to memory that's already been freed, or
> otherwise to an object which no longer exists. Another common source
> would be that the pointer object itself got overwritten such that it's
> no longer a meaningful pointer.


Function list_remove_element_by_id() was one which was causing problems,
I think now it is working fine. See any problem ?




#include <stdio.h>
#include <stdlib.h>
#include <string.h>


struct sq_struct
{
char title[100+1];
struct sq_struct* next;
};


struct sq_list
{
struct sq_struct* head;
struct sq_struct* tail;
};

struct sq_struct* list_add_element( struct sq_list*, const char* t);
struct sq_list* list_remove_element(struct sq_list*);
struct sq_list* list_remove_element_by_id(struct sq_list*, const char* );


struct sq_list* list_new(void);
struct sq_list* list_free( struct sq_list* );

void list_print( const struct sq_list* );
void list_print_element(const struct sq_struct* );



int main(void)
{
struct sq_list* mt = NULL;
struct sq_struct* tmp = malloc(1 * sizeof(*tmp));

if(NULL == tmp)
{
printf("Out of Memory\n");
exit(EXIT_FAILURE);
}

mt = list_new();

list_add_element(mt, "1");
list_add_element(mt, "2");
list_add_element(mt, "3");
list_add_element(mt, "4");

list_print(mt);

list_remove_element_by_id(mt, "3");
list_remove_element_by_id(mt, "1");


list_print(mt);
list_free(mt); /* always remember to free() the malloc()ed memory */
free(mt); /* free(), list is kept separate from free()ing the
structure, I think its a good design */
mt = NULL; /* after free() always set that pointer to NULL, C
will run havoc on you if you try to use a dangling pointer */

list_print(mt);
free(tmp);
printf("everything free()ed\n");
return 0;
}


/* Will always return the pointer */
struct sq_struct* list_add_element(struct sq_list* s, const char* t)
{
struct sq_struct* retp;
struct sq_struct* p;

if(NULL == s || NULL == t)
{
fprintf(stderr, "IN: %s @%d: NULL Args", __FILE__, __LINE__);
return NULL;
}

p = malloc(1 * sizeof *p);
if( NULL == p )
{
fprintf(stderr, "IN %s, %s: malloc() failed\n", __FILE__,
"list_add");
retp = NULL;
}
else
{
strcpy(p->title, t);
p->next = NULL;

if( NULL == s->head && NULL == s->tail )
{
/* printf("Empty list, adding p->num: %d\n\n", p->num); */
s->head = s->tail = p;
retp = p;
}
else if( NULL == s->head || NULL == s->tail )
{
fprintf(stderr, "There is something seriously wrong with your
assignment of head/tail to the list\n");
free(p);
retp = NULL;
}
else
{
/* printf("List not empty, adding element to tail\n"); */
s->tail->next = p;
s->tail = p;
retp = p;
}
}

return retp;
}


struct sq_list* list_remove_element(struct sq_list* s)
{
struct sq_list* retp;
struct sq_struct* h;
struct sq_struct* nx;

if(NULL == s)
{
printf("IN: %s @%d: Invalid Args\n", __FILE__, __LINE__);
retp = NULL;
}
else if(NULL == s->head)
{
printf("IN: %s %d: Empty List\n", __FILE__, __LINE__);
retp = s;
}
else
{
h = s->head;
nx = h->next;
printf("Removing %s\n", h->title);
s->head = nx;
free(h);
if(NULL == s->head) s->tail = s->head; /* Means last element was
removed */
retp = s;
}

return retp;
}





/* Ugliest of the Hacks I have evr done. Queue (FIFO) is not the right
data structure to use if I am
removing elements from anywhere but top */
struct sq_list* list_remove_element_by_id(struct sq_list* s, const char*
title )
{
struct sq_struct* h;
struct sq_struct* prv;
struct sq_struct* nx;

if( NULL == s || NULL == title)
{
printf("IN: %s @%d: Invalid Args\n", __func__, __LINE__);
return NULL;
}
else if( NULL == s->head && NULL == s->tail )
{
printf("IN: %s @%d: Well, List is empty\n", __func__, __LINE__);
return NULL;
}
else if( NULL == s->head || NULL == s->tail )
{
printf("IN: %s @%d: ", __func__, __LINE__);
printf("There is something seriously wrong with your list\n");
printf("One of the head/tail is empty while other is not \n");
return NULL;
}

for(h = s->head, prv = NULL; h; h = h->next)
{
if(NULL == h->title)
{
printf("IN: %s @%d: *ERROR* struct exists but ID is NULL :-/",
__func__, __LINE__);
}
else
{
if(0 == strcmp(h->title, title))
{
printf("IN: %s @ %d: Removing %s\n", __FILE__, __LINE__, h-
>title);

nx = h->next;
free(h);
if(NULL == prv) /* means we are deleting head */
{
s->head = nx;
}
else
{
prv->next = nx;
}
break;
}
}

prv = h;
}

return s;
}


/* ---------------------- small helper fucntions
---------------------------------- */
struct sq_list* list_free( struct sq_list* s )
{
if(NULL == s)
{
printf("NULL args, nothing to free\n");
return NULL;
}

while( s->head )
{
list_remove_element(s);
}

return s;
}

struct sq_list* list_new(void)
{
struct sq_list* p = malloc( 1 * sizeof(*p));

if( NULL == p )
{
printf("LINE: %d, malloc() failed\n", __LINE__);
}

p->head = p->tail = NULL;

return p;
}

void list_print( const struct sq_list* ps )
{
struct sq_struct* p = NULL;

if( ps )
{
for( p = ps->head; p; p = p->next )
{
list_print_element(p);
}
}

printf("------------------\n");
}

void list_print_element(const struct sq_struct* p )
{
if( p )
{
printf("title: %s\n", p->title);
}
else
{
printf("Can not print NULL struct \n");
}
}



=========================== OUTPUT ============================
[arnuld@dune programs]$ gcc -ansi -pedantic -Wall -Wextra strange-queue.c
[arnuld@dune programs]$ ./a.out
title: 1
title: 2
title: 3
title: 4
------------------
IN: strange-queue.c @ 186: Removing 3
IN: strange-queue.c @ 186: Removing 1
title: 2
title: 4
------------------
Removing 2
Removing 4
------------------
everything free()ed
[arnuld@dune programs]$






--
http://www.lispmachine.wordpress.com
 
Reply With Quote
 
Seebs
Guest
Posts: n/a
 
      01-11-2011
On 2011-01-11, arnuld <(E-Mail Removed)> wrote:
> I solved the problem before I read your post and reason is *exactly* what
> you stated. Pointer was getting overwritten. How ? , well, function-1
> calls function-2 which calls functions-3 and which in turn calls
> function-4 and function-4 has a malloc() which was never free()ed, so it
> was continually allocating memory. I guess at some point it was
> overwriting other pointers.


That is not how it works. This is incoherent. Allocating things
without freeing them can cause malloc() to fail. On some systems, it can
cause your program to be killed. But it can't, ever, "overwrite other
pointers".

What you're saying doesn't even begin to make sense. There is no grand
list of all the pointers in your program so something like malloc could
overwrite them. The only way pointers get overwritten is when some other
object near them gets overwritten, or when you assign values to them.

And here's the thing: If you did something, and the problem got better,
but your theory about what happened is this incoherent, the chances are
very good that you have merely moved the problem, not actually corrected
it.

And I assure you, while a function which allocates but never frees can cause
some problems, "overwriting other pointers" is not one of them. Something
else was causing the overwrites, even if you haven't found it yet. Or your
understanding of what that function was doing is wrong.

> If I should not do the work then I think majority of programmers in
> Indian corporate should resign because I have programmers who earn 2 to 8
> times more than me and they always trust gdb more than putting error
> checks in program.


Could be. But if they can write code which works, and you can't, they're
earning more than you are.

> I have yet to meet a programmers who checks the return
> value of strtoul(), they says its a waste to do such things, business
> reasons are more important e.g. deadline and developing code faster. I
> agree that I am quite incompetent as compared to programmers at clc and I
> am happy for that because I get to learn from people like you


I usually don't check that, but I pretty often check that the string did
in fact end, and didn't have trailing garbage.

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / (E-Mail Removed)
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
I am not speaking for my employer, although they do rent some of my opinions.
 
Reply With Quote
 
Seebs
Guest
Posts: n/a
 
      01-11-2011
On 2011-01-11, arnuld <(E-Mail Removed)> wrote:
> Function list_remove_element_by_id() was one which was causing problems,
> I think now it is working fine. See any problem ?


I don't feel like reading 300 lines of bad code on the off chance that
there's still a problem.

Try this: Create a minimal reproducer for the segfaults, *without* fixing
them. Strip out everything you can -- you should be able to get it shorter
than this.

Then put in your proposed fix using #ifdefs so you have a program which,
with -DARNULD works, and without it segfaults, and we could have a look
at it.

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / (E-Mail Removed)
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
I am not speaking for my employer, although they do rent some of my opinions.
 
Reply With Quote
 
Ike Naar
Guest
Posts: n/a
 
      01-11-2011
On 2011-01-11, arnuld <(E-Mail Removed)> wrote:
> [snip]
> struct sq_struct
> {
> char title[100+1];
> struct sq_struct* next;
> };
> [snip]
> int main(void)
> {
> struct sq_struct* tmp = malloc(1 * sizeof(*tmp));
>
> if(NULL == tmp)
> {
> printf("Out of Memory\n");
> exit(EXIT_FAILURE);
> }


What's the purpose of ``tmp''? It's allocated, never used, then
deallocated.

> [snip]
> struct sq_list* list_remove_element_by_id(struct sq_list* s, const char*
> title )
> {
> [snip]
> if(NULL == h->title)


Why this check? h->title is an array, it cannot be NULL.
 
Reply With Quote
 
arnuld
Guest
Posts: n/a
 
      01-11-2011
> On Tue, 11 Jan 2011 10:41:03 +0000, Ike Naar wrote:

> On 2011-01-11, arnuld <(E-Mail Removed)> wrote:


>> int main(void)
>> {
>> struct sq_struct* tmp = malloc(1 * sizeof(*tmp));
>>
>> if(NULL == tmp)
>> {
>> printf("Out of Memory\n");
>> exit(EXIT_FAILURE);
>> }


> What's the purpose of ``tmp''? It's allocated, never used, then
> deallocated.


leftover from earlier debugging session. Removed.



>> struct sq_list* list_remove_element_by_id(struct sq_list* s, const
>> char* title )
>> {
>> [snip]
>> if(NULL == h->title)


> Why this check? h->title is an array, it cannot be NULL.


Its an array I know and it will always be passed as a pointer in function
argument and pointer can be NULL.




--
http://www.lispmachine.wordpress.com
 
Reply With Quote
 
arnuld
Guest
Posts: n/a
 
      01-11-2011
> On Tue, 11 Jan 2011 10:54:25 +0000, arnuld wrote:

> Its an array I know and it will always be passed as a pointer in
> function argument and pointer can be NULL.


Eh.. wrong thought. Corrected.





--
http://www.lispmachine.wordpress.com
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
invoking a segfault within a segfault handler - is this undefinedbehavior? Andrey Vul C Programming 8 07-30-2010 02:14 PM
Benchmark segfault [Was: Array#inject to create a hash versus Hash[*array.collect{}.flatten] ] Michal Suchanek Ruby 6 06-13-2007 04:40 AM
Array#inject to create a hash versus Hash[*array.collect{}.flatten] -- Speed, segfault Anthony Martinez Ruby 4 06-11-2007 08:16 AM
Copying file lines into an array [ newbie ] inyc163 C Programming 2 05-25-2004 09:09 PM
Copying file lines into an array [ newbie ] jj C Programming 4 05-24-2004 05:19 PM



Advertisments