Segfault - strcpy() copying into array

A

arnuld

I don't get it why I am getting Segmentation Fault here in strcpy():


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


struct myStruct
{
char title[10];
};



void checkArgument(struct myStruct *p);


int main(void)
{
struct myStruct *st;

strcpy(st->title, "clc");
checkArgument(st);

return 0;
}


void checkArgument(struct myStruct *p)
{
if(NULL == p)
{
printf("*ERROR* - Invalid Args\n");
}
else if('\0' == p->title[0])
{
printf("Empty member\n");
}
else
{
printf("Title = %s\n", p->title);
}
}
 
O

Owen Jacobson

I don't get it why I am getting Segmentation Fault here in strcpy():

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

struct myStruct
{
  char title[10];

};

void checkArgument(struct myStruct *p);

int main(void)
{
  struct myStruct *st;

This line declares a variable of type "pointer to struct myStruct" but
does not initialize it.
  strcpy(st->title, "clc");

This line dereferences the uninitialized variable. Boom, segfault (or
any other possible result).
 
S

Seebs

I don't get it why I am getting Segmentation Fault here in strcpy():

I am starting to wonder whether you should just give up on C.
struct myStruct
{
char title[10];
};
int main(void)
{
struct myStruct *st;
strcpy(st->title, "clc");

What does st point to?

If this two-line bug doesn't leap out at you, after the number of times
you've gone through this, maybe this is not the right language for you.

It's one thing not to understand this stuff when you've never seen it
before, but you've had MANY questions which involved uninitialized pointers,
and any compiler I've used in the last decade or two would have warned
you about this if you had it configured sanely.

-s
 
A

arnuld

I am starting to wonder whether you should just give up on C.

I won't :)

struct myStruct
{
char title[10];
};
int main(void)
{
struct myStruct *st;
strcpy(st->title, "clc");

What does st point to?

If this two-line bug doesn't leap out at you, after the number of times
you've gone through this, maybe this is not the right language for you.

It's one thing not to understand this stuff when you've never seen it
before, but you've had MANY questions which involved uninitialized
pointers, and any compiler I've used in the last decade or two would
have warned you about this if you had it configured sanely.

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


[arnuld@dune downloads]$ gcc --version
gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.


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.
 
I

Ike Naar

I don't get it why I am getting Segmentation Fault here in strcpy():
[...]
int main(void)
{
struct myStruct *st;

st has type ``pointer to struct myStruct''.
st is an uninitialized pointer; it does not point to a struct myStruct object.
strcpy(st->title, "clc");

Here you try to dereference an indeterminate pointer ``st''.
 
J

jacob navia

Le 10/01/11 12:37, Richard a écrit :
Simple answer : step through with a DEBUGGER.

The line will leap out at you.

Sheesh.

DEBUGGER?

Please do not use 8 letter words here in comp.lang.c!

They are twice as nasty as 4 letter words.
 
F

Francois Grieu

On 10/01/2011 11:06, arnuld wrote
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
that code does have a malloc() with a null check.

Perhaps st->title was never set to a valid non-NULL pointer,
or was set to the result of malloc() for zero length.

Francois Grieu
 
F

Francois Grieu

Francois Grieu said:
On 10/01/2011 11:06, arnuld wrote
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
that code does have a malloc() with a null check.

Perhaps st->title was never set to a valid non-NULL pointer,
or was set to the result of malloc() for zero length.

Francois Grieu

Did you not see the other correct answers?

Yes I did, but I'm trying to solve the REAL problem
of the Original Poster as summarized in the text I quoted,
NOT what he stated in his irrelevant original post.

Francois Grieu
 
K

Keith Thompson

arnuld said:
I am starting to wonder whether you should just give up on C.

I won't :)

struct myStruct
{
char title[10];
};
int main(void)
{
struct myStruct *st;
strcpy(st->title, "clc");

What does st point to?

If this two-line bug doesn't leap out at you, after the number of times
you've gone through this, maybe this is not the right language for you.

It's one thing not to understand this stuff when you've never seen it
before, but you've had MANY questions which involved uninitialized
pointers, and any compiler I've used in the last decade or two would
have warned you about this if you had it configured sanely.

gcc -ansi -pedantic -Wall -Wextra does not give any warning:
[...]

gcc typically doesn't recognize uninitialized variables unless it's
invoked with optimization. The kind of analysis needed to detect the
error is also necessary to perform many optimizations.

Adding "-O1" to the above produces a warning.

(Other compilers are likelyi to behave similarly.)
 
S

Seebs

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
 
A

arnuld

...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 :)
 
A

arnuld

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.
 
L

luser- -droog

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

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.
 
A

arnuld

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-
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
 
S

Seebs

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
 
S

Seebs

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
 
I

Ike Naar

[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.
 
A

arnuld

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.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,007
Latest member
obedient dusk

Latest Threads

Top