What's wrong with C?

[Greg White]

Hi:

I have a string function that is not working as I expect.


char *makefname(fnum)
int fnum;
{
char fname[80];
int pid = 0;
char *fname_ptr;


pid = getpid();
sprintf(fname,"/tmp/Tfile%d.%d.DAT",pid,fnum);
printf("fnum = %d fname = %s\n",fnum,fname);
fname_ptr = fname;
return(fname_ptr);
}

int main()
{
char *fname1, *fname2, *fname3, *fname4;
fname1 = makefname(1);
fname2 = makefname(2);
fname3 = makefname(3);
fname4 = makefname(4);
printf("fname1 = %s\n",fname1);
printf("fname2 = %s\n",fname2);
printf("fname3 = %s\n",fname3);
printf("fname4 = %s\n",fname4);
}



No matter what I do only the last value is displayed but everything is fine inside the makefname function. I feel like I am making a very simple mistake but I have been working on this for over two hours!!

Thanks in advance.

[A. Clay Stephenson · Accepted solution]

Hi Pascal:

The answer to your question is that it's doing just what you tell it. You actually have 2 serious problems and a few small ones.
Serious problem 1:) Your string function is returning a pointer to an auto declared variable. This code is actually working by accident. fname should be declared as a static char fname[80] - to preserve the data ourtside the function call. This will not fix your problem about only the last invocation counting but it will make your code SAFE.

Serious Problem 2:) You are returning a POINTER; it points to the same area there fore only the last invocation counts. You need to either dynamically allocate the space each time or set up a round-robin buffer so that the buffer is only re-used after say 5 invocations when presumably you have copied the data to other variables.


Clay

[harry d brown jr]

Get some memory - malloc
add the proper library
copy fname to memory loc

#
#include
#
char *makefname(fnum)
int fnum;
{
char fname[80];
int pid = 0;
char *fname_ptr;

#get some memory
fname_ptr = malloc(sizeof(fname));
pid = getpid();
sprintf(fname,"/tmp/Tfile%d.%d.DAT",pid,fnum);
printf("fnum = %d fname = %s\n",fnum,fname);
#save fname
strcpy(fname_ptr,fname);
return(fname_ptr);
}

[A. Clay Stephenson]

Okay Pascal:

I've attached the round-robin version first; it will cycle through 8 calls before reusing the space. Minor quibble: Allocate temp files from /var/tmp'; /tmp should only be used by OS utils; /var/tmp fills up -> inconvenient; /tmp fills up -> BAD.

Nore that you really don't need fname_ptr, the array address can be returned. Also, no need to call getpid more than once. Let a static varaiable store this to make your code more efficient. It is very importtant that both the character array and the current counter be declared as static.

Clay

[A. Clay Stephenson]

Hi again:

Here's the somewhat better solution:

Malloc a chuck of memory each time but don't forget to free it when you are done.

[A. Clay Stephenson]

Finally Pascal:

What are you writing this function for anyhow?

tmpnam() and tempnam() are ready made functions which already do this. If this was just a snippet to illustrate a problem with strings then ok but otherwise use the built-ins. Man tempnam for details. Note the comments about returning a static buffer if a NULL pointer is supplied which is really just like your problem.

You should also look at the tmpfile function. It creates a file and them immediately unlinks it. There is no directory entry but you can do amnythink you like to the file until you close it. The file then disappears automatically.
Man tmpfile(3c) for details.

Clay

[Greg White]

Thanks Clay and Harry.