LinuxQuestions.org
Download your favorite Linux distribution at LQ ISO.
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices

Reply
 
Search this Thread
Old 04-02-2004, 10:09 PM   #1
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Rep: Reputation: 45
strcpy causing segfault


All, can anyone please explain to me why the following code is seg faulting on the strcpy?

buf[0] = '\0';
s_element->data_value = (totalused / total)*100;
sprintf(buf, "CPU 0\nLoad: %3.2f%%", s_element->data_value);
if ((s_element->t_element.str = malloc(strlen(buf))) != NULL)
strcpy(s_element->t_element.str, buf);



The code is contained in a function that executes approximately every 3 seconds. buf is a member variable of the function with type 'static char[256]'.

s_element is a pointer to a global static struct.

I'm trying to format data (s_element's data_value) into a string to be displayed. I sprintf it into a buffer then allocate space for s_element's char*.

the call to strcpy executes twice and on the third execution (the third time the function is called) it seg faults.

Can anyone lend a hand?,
Thanks,
jpbarto
 
Old 04-02-2004, 10:15 PM   #2
leonscape
Senior Member
 
Registered: Aug 2003
Location: UK
Distribution: Debian SID / KDE 3.5
Posts: 2,313

Rep: Reputation: 47
These are null terminated strings. and strlen doesn't count the null. So

if ((s_element->t_element.str = malloc(strlen(buf) + 1)) != NULL)
strcpy(s_element->t_element.str, buf);
 
Old 04-02-2004, 10:49 PM   #3
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
Thanks, but I'm afraid that didn't do it.
I changed the code a bit for debugging purposes:

new code:
s_element->data_value = (totalused / total)*100;
sprintf(buf, "CPU 0 Load: %3.2f%%", s_element->data_value);
fprintf(stderr, "buf is %d\n", strlen(buf));
fprintf(stderr, "buf contains %s\n", buf);
if ((s_element->t_element.str = malloc(strlen(buf)+1)) != NULL){
fprintf(stderr, "str is %d\n", strlen(s_element->t_element.str));
/*strcpy(s_element->t_element.str, buf);
fprintf(stderr, "str contains %s\n", s_element->t_element.str);*/
}

And the resulting output:
buf is 18
buf contains CPU 0 Load: 11.24%
str is 22
buf is 19
buf contains CPU 0 Load: 100.00%
str is 10
buf is 17
buf contains CPU 0 Load: 3.67%
str is 10
buf is 17
buf contains CPU 0 Load: 0.67%
str is 10
buf is 17
buf contains CPU 0 Load: 1.33%
str is 10

Why is malloc creating a string that is 10 when its told to create one that is 17? Or maybe the question should be, where is the null terminator coming from?
 
Old 04-02-2004, 11:18 PM   #4
leonscape
Senior Member
 
Registered: Aug 2003
Location: UK
Distribution: Debian SID / KDE 3.5
Posts: 2,313

Rep: Reputation: 47
sprintf will append the null terminator for you, on buf.

Dont forget str is pointing at assigned memory which could contain anything ( you haven't set it yet ) So the values from

fprintf(stderr, "str is %d\n", strlen(s_element->t_element.str));

are meaningless.

My advice would not be to use this method. set s_element->t_element.str as an array of 25 characters. ( Since this covers the maximum quite confortably. ) and drop the malloc all together. Also are you freeing this memory?
 
Old 04-03-2004, 12:19 AM   #5
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
I'm not freeing the memory (perhaps realloc would be better than malloc?) and I wish that I could set s_element->t_element.str to a static array length but its a member of a struct and that struct is used for multiple things (receiving output from 'who' for example).

Ok, so strlen is useless until s_element->t_element.str is set, I can understand that. However why then is the program crashing on the second strcpy? Is it something to do with calling malloc to assign memory to the same pointer without ever freeing the pointer?

Thanks for all your help thus far by the way,
there are similar lines of code in other functions that have given me no problem. In those instances 'buf' is populated by a gets() where as here it is populated by a sprintf... its odd.
 
Old 04-03-2004, 12:37 AM   #6
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
leonscape, I have to apologize. It seems I may have been chasing the wrong code. Although running the app doesn't print out a specific line of code as the culprit (it in fact returns an error for chunk_alloc in libc.so.6). I inserted multiple printf statements to trace the app as it flows and it segfaults on an XSync call... odd...

Any recomendations on how I could find out more about the error?

Thanks again for all your help with this,
jpbarto
 
Old 04-03-2004, 10:27 AM   #7
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
Alright, some more studying shows that it segfauls on the XSync function (which occurs in another one of my defined functions) but XSync only segfaults if I perform the strcpy.

And buf (of the original posted code) is not the culprit as a strcpy of
strcpy(s_element->t_element.str, "cpu 0\nload: 9.3%");

causes a segfault as well. Is there some attribute of how strcpy functions that I'm forgetting?
 
Old 04-03-2004, 02:32 PM   #8
leonscape
Senior Member
 
Registered: Aug 2003
Location: UK
Distribution: Debian SID / KDE 3.5
Posts: 2,313

Rep: Reputation: 47
What is the call to xsync, are you clearing the queue?

Is the display your calling it on, anything to do with s_element.
 
Old 04-04-2004, 01:17 AM   #9
The_Nerd
Member
 
Registered: Aug 2002
Distribution: Debian
Posts: 540

Rep: Reputation: 32
Malloc?

Ok! The biggest problem that I always run into when using malloc/realloc/free is that I forget to #include <malloc.h> check this first

also you are not adding memory space for the trailing '\0' (+1)

if you are indeed including malloc.h then change your code as follows:

s_element->t_element.str = realloc(s_element->t_element.str, (strlen(buf)+1)*sizeof(unsigned char));
if (s_element->t_element.str) strcpy(s_element->t_element.str, buf);

Someone please correct my if I am wrong, but won't

if ( (s_element->t_element.str = malloc(strlen(buf)) ) != NULL)

always be true? Aren't we saying here "if (assignment) == true) then --"
and the assignment will always be true... will it not? It is safer (I think) to make the if statement seperate from the assignment statement.

Don't use malloc in an operation like that. Use mypt=realloc(mypt, mylen).

mylen + 1 if it is a null terminated string

MAKE SURE THAT s_element->t_element.str IS SET TO NULL BEFORE REALLOC IS CALLED FOR THE FIRST TIME... APPON STARTUP

Hope that helps
 
Old 04-04-2004, 06:05 PM   #10
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
Everyone, thanks for the help.

XSync is being called (this is an XScreensaver hack) before the next iteration of the program... I don't know, it was part of the base 'skeleton' hack that I began modding for my own purposes.

Also, Nerd, I tried your suggestion and unfortunately the app still crashed. Althought as I've been playing with this I've noticed some odd but yet consistent behavior.

The following code fails with a seg fault:
sprintf(buf, "CPU 0\nLoad: %3.2f%%", s_element->data_value);
s_element->t_element.str = realloc(s_element->t_element.str,
(strlen(buf) + 1)*sizeof(unsigned char));
if (s_element->t_element.str)
strcpy (s_element->t_element.str, buf);

But this code works just fine:
sprintf(buf, "CPU 0\n %3.2f%%", s_element->data_value);
s_element->t_element.str = realloc(s_element->t_element.str,
(strlen(buf) + 1)*sizeof(unsigned char));
if (s_element->t_element.str)
strcpy (s_element->t_element.str, buf);

If you notice the only difference is the length of the string stored in 'buf'. I even hardcoded malloc (and realloc) to allocate 24 bytes and got the same behavior, so t_element.str allocated with malloc(24) would fail if 'Load' was copied into str but if I reduced the lenght of the string it works ok... any explanations?

jpbarto
 
Old 04-04-2004, 07:36 PM   #11
leonscape
Senior Member
 
Registered: Aug 2003
Location: UK
Distribution: Debian SID / KDE 3.5
Posts: 2,313

Rep: Reputation: 47
The original construct is not wrong, Its used quite a lot, what your saying is

if (( the value assigned) == true )

Thats what the extra brackets are for. Without them then yes it would always return true, with the brackets it means something slightly diffrent.

Now I have though of something, I hadn't though before ( it has been a long while since I used c ) theirs no cast from void*

s_element->t_element.str = (char *)realloc(s_element->t_element.str, (strlen(buf) + 1));

malloc and realloc, return a void pointer, since I think str is a char pointer you need a cast.
this is probably causing the crash. As you maybe corrupting the stack.
 
Old 04-05-2004, 08:12 PM   #12
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
leonscape, thanks for the continued advice; but still no dice. I tell you guys what, my next post is going to layout the entire contents of the structs used, and the function using them, perhaps there is something peripheral causing this.
 
Old 04-05-2004, 08:15 PM   #13
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
SysInfo.c

typedef struct {
/* string to display */
char *str;
/* string length, of course */
int slen;
/* position (in percentage) to place string */
/* where top-left-most of window is 0.0,0.0 and */
/* bottom-right-most of window is 1.0,1.0 */
float pos_x;
float pos_y;
} txt_element;

typedef struct {
Pixmap *img;
float pos_x;
float pos_y;
} img_element;

typedef struct {
txt_element t_element;
img_element i_element;
/* command to populate element */
char *cmd;
/* if no command, then a custom function to populate the element */
void (*func) (void *);
/* double var to store a reading (to be displayed by a guage) */
double data_value;
/* will the string change over the course of the apps lifetime */
BOOL constant;
} scr_element;


static scr_element cpuload = {{NULL,0,0.05,0.4},{NULL,0.05,0.5},NULL,&populate_cpu_load,0,FALSE};


static void
populate_cpu_load (scr_element *s_element)
{
int fd, len;
static char buf[BUFSIZE];
double diffuser, diffnice, diffsys, diffidle, total, totalused;

/* copy over the old *fresh* values to prep for reading new ones */
memcpy(&cpuload_last, &cpuload_fresh, sizeof(cpuload_last));

/* read in the /proc/stat cpu values */
if ((fd = open("/proc/stat", 00))){
len = read(fd, buf, sizeof(buf)-1);
close(fd);

buf[len] = '\0';
len = sscanf(buf, "cpu %ld %ld %ld %ld\n", &cpuload_fresh[0],
&cpuload_fresh[1],
&cpuload_fresh[2],
&cpuload_fresh[3]);
}else
fprintf(stderr, "%s: Failed to open /proc/stat for reading\n", progname);

diffuser = cpuload_fresh[0] - cpuload_last[0];
diffnice = cpuload_fresh[1] - cpuload_last[1];
diffsys = cpuload_fresh[2] - cpuload_last[2];
diffidle = cpuload_fresh[3] - cpuload_last[3];
total = diffuser + diffnice + diffsys + diffidle;
totalused = diffuser + diffnice + diffsys;

s_element->data_value = (totalused / total)*100;
sprintf(buf, "CPU 0\n%3.2f%%", s_element->data_value);
s_element->t_element.str = realloc(s_element->t_element.str,
(strlen(buf) + 1)*sizeof(unsigned char));
if (s_element->t_element.str)
strcpy (s_element->t_element.str, buf);

if (s_element->data_value > 83)
s_element->i_element.img = cguages[6];
if (s_element->data_value > 67)
s_element->i_element.img = cguages[5];
if (s_element->data_value > 50)
s_element->i_element.img = cguages[4];
if (s_element->data_value > 33)
s_element->i_element.img = cguages[3];
if (s_element->data_value > 16)
s_element->i_element.img = cguages[2];
if (s_element->data_value > 0)
s_element->i_element.img = cguages[1];
else
s_element->i_element.img = cguages[0];
}


then there is a 'main' function which calls the 'populate' function approximately every three seconds in order to maintain present CPU load.

ideas?

Thanks for the continuing help on this, I really appreciate it.
jpbarto

Oh, P.S., cguages is just an array of pointers to Pixmaps.
 
Old 04-06-2004, 01:06 AM   #14
The_Nerd
Member
 
Registered: Aug 2002
Distribution: Debian
Posts: 540

Rep: Reputation: 32
Whats BUFSIZE?
 
Old 04-06-2004, 08:50 AM   #15
jpbarto
Senior Member
 
Registered: Mar 2003
Location: Pittsburgh, PA
Distribution: Gentoo / NetBSD
Posts: 1,251

Original Poster
Rep: Reputation: 45
oh, my bad! It is
#define BUFSIZE 2600
 
  


Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

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 Off
HTML code is Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
best way to avoid buffer overflow... with strcpy and strcat os2 Programming 4 03-02-2005 03:24 PM
diff between strcpy and strncpy djgerbavore Programming 11 08-04-2004 08:01 AM
strcpy problem rajatgarg Programming 5 11-20-2003 01:46 AM
question with strcpy Jo_Nak Programming 1 07-02-2003 05:23 PM
strcpy to an array of strucs gyroWang Programming 4 03-22-2003 11:01 PM


All times are GMT -5. The time now is 01:09 AM.

Main Menu
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
identi.ca: @linuxquestions
Facebook: linuxquestions Google+: linuxquestions
Open Source Consulting | Domain Registration