question about memory allocation 
Author Message
 question about memory allocation

I have a program I think should cause a seg. fault but doesn't. I started
with a question about how I was freeing memory I had reallocated. The
program in question uses some non-ansi c functions and corresponding header
so I tried to frame the question with a compliant program and got a core
dump. I worked on it to get at the error in my thinking and got to the bit
below - which seems to me is probably the correct way to allocate the memory
I wanted. The thing is the program I was originally working on works as
expected under FreeBSD compiled with gcc and while it doesn't work as
planned under Win98 (compiled with gcc - from the cygwin package) it also
doesn't seg. fault. With apologies I've included both programs for comment.
I suspect gcc has somehow let me get away with something here and if that is
the case I don't want to develop a bad habit. Right after this I'll write
myself an admonishing note for include non-ansi functions in my posting ;-)
TIA and regards,
Greg Martin

/* This seems right to me */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAXSIZE 80

int main(void)
{
 int cnt;
 int i;
 char *info[MAXSIZE];
 char someinfo[MAXSIZE];

 cnt = 0;
 while((fgets(someinfo, MAXSIZE-1, stdin)))
 {
  if(((strcmp(someinfo, "\n")) == 0)) break;
  ++cnt;
  if((info[cnt-1] = malloc(strlen(someinfo))) == NULL)
  {
   puts("realloc() failed");
   exit(EXIT_FAILURE);
  }

  strcpy(info[cnt-1], someinfo);
  info[cnt-1][strlen(info[cnt-1])-1] = '\0';
 }

 for (i = 0; i < cnt; ++i)
 {
  puts(info[i]);
  free(info[i]);
 }
 return 0;

Quote:
}

/* This is the one I don't think should work! (though I did when I wrote it)
*/

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

int main(void)
{
        struct dirent *pdirstruct;
        const char basedir[] = "/usr/home/gregm/public_html";
        int cnt, i;
        DIR *dp;
        char *pdir[FILENAME_MAX];

        cnt = 0;
        if((dp = opendir(basedir)) != NULL)
        {
                while((pdirstruct = readdir(dp)))
                {
                        ++cnt;
                        if((*pdir = realloc(*pdir, sizeof(pdir) *
cnt))==NULL)
                        {
                                puts("realloc() failed");
                                exit(0);
                        }
                        pdir[cnt] = pdirstruct->d_name;
                }
        }
        else
        {
                puts("opendir(failed)");
                exit(0);
        }
        for (i = 0; i < cnt; ++i)
        {
                puts(pdir[i]);
        }
        free(*pdir);
        return 0;

Quote:
}



Wed, 28 Nov 2001 03:00:00 GMT  
 question about memory allocation


Quote:
> I have a program I think should cause a seg. fault but doesn't. I
started
> with a question about how I was freeing memory I had reallocated. The
> program in question uses some non-ansi c functions and corresponding
header
> so I tried to frame the question with a compliant program and got a
core
> dump. I worked on it to get at the error in my thinking and got to the
bit
> below - which seems to me is probably the correct way to allocate the
memory
> I wanted. The thing is the program I was originally working on works
as
> expected under FreeBSD compiled with gcc and while it doesn't work as
> planned under Win98 (compiled with gcc - from the cygwin package) it
also
> doesn't seg. fault. With apologies I've included both programs for

comment.

You are never guaranteed a segment fault; that is one possible response
to undefined behavior.  (Most people are stuck with programs that do
have segment faults that they think shouldn't occur, so perhaps you're
fortunate.)

- Show quoted text -

Quote:
> /* This seems right to me */
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>

> #define MAXSIZE 80

> int main(void)
> {
>  int cnt;
>  int i;
>  char *info[MAXSIZE];
>  char someinfo[MAXSIZE];

>  cnt = 0;
>  while((fgets(someinfo, MAXSIZE-1, stdin)))
>  {
>   if(((strcmp(someinfo, "\n")) == 0)) break;
>   ++cnt;
>   if((info[cnt-1] = malloc(strlen(someinfo))) == NULL)
>   {
>    puts("realloc() failed");
>    exit(EXIT_FAILURE);
>   }

>   strcpy(info[cnt-1], someinfo);
>   info[cnt-1][strlen(info[cnt-1])-1] = '\0';
>  }

>  for (i = 0; i < cnt; ++i)
>  {
>   puts(info[i]);
>   free(info[i]);
>  }
>  return 0;
> }

The first program contains no check that cnt doesn't reach MAXSIZE,
so you may overrun the array info.  You should malloc the string
length + 1 if you're going to copy a string (the most serious error).
fgets() might not include the '\n' if the line is too long, which
should be considered even if not checked.  ("realloc failed\n" is
misleading, since it's actually malloc called.)

I didn't see any other problems with the first program (I'd be
thoroughly unsurprised if I missed glaring errors), and laziness
masquerading as pedantic purity stopped me before I looked at the
second.

--
MJSR

Sent via Deja.com http://www.deja.com/
Share what you know. Learn what you don't.



Wed, 28 Nov 2001 03:00:00 GMT  
 question about memory allocation

wrote in comp.lang.c:

<Jack>

Where to begin?  I haven't exhaustively analyzed your programs, but
there are some errors I have found, see below.

Quote:
> I have a program I think should cause a seg. fault but doesn't. I started
> with a question about how I was freeing memory I had reallocated. The
> program in question uses some non-ansi c functions and corresponding header
> so I tried to frame the question with a compliant program and got a core
> dump. I worked on it to get at the error in my thinking and got to the bit
> below - which seems to me is probably the correct way to allocate the memory
> I wanted. The thing is the program I was originally working on works as
> expected under FreeBSD compiled with gcc and while it doesn't work as
> planned under Win98 (compiled with gcc - from the cygwin package) it also
> doesn't seg. fault. With apologies I've included both programs for comment.
> I suspect gcc has somehow let me get away with something here and if that is
> the case I don't want to develop a bad habit. Right after this I'll write
> myself an admonishing note for include non-ansi functions in my posting ;-)
> TIA and regards,
> Greg Martin

> /* This seems right to me */
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>

> #define MAXSIZE 80

> int main(void)
> {
>  int cnt;
>  int i;
>  char *info[MAXSIZE];
>  char someinfo[MAXSIZE];

>  cnt = 0;
>  while((fgets(someinfo, MAXSIZE-1, stdin)))

You don't need to use MAXSIZE-1 here.  fgets() will only accept one
less than the specified size from the stream to leave room for the
'\0' automatically.

Quote:
>  {
>   if(((strcmp(someinfo, "\n")) == 0)) break;

Just an efficiency item.  I would have used the following to do the
same thing without a function call:

   if (*someinfo == '\n') break; /* or someinfo [0] */

Quote:
>   ++cnt;

Any reason not to move the ++cnt to the bottom of the loop and save
all of the cnt-1 references?

Quote:
>   if((info[cnt-1] = malloc(strlen(someinfo))) == NULL)

This is a real problem.  When you copy a string the destination must
contain at least strlen(string) + 1 characters, because the value
returned by strlen() does NOT count the '\0'.

Quote:
>   {
>    puts("realloc() failed");
>    exit(EXIT_FAILURE);
>   }

>   strcpy(info[cnt-1], someinfo);

Since you didn't use strlen()+1 in your malloc(), this copy ends up
writing a '\0' past the end of the allocated memory.

- Show quoted text -

Quote:
>   info[cnt-1][strlen(info[cnt-1])-1] = '\0';
>  }

>  for (i = 0; i < cnt; ++i)
>  {
>   puts(info[i]);
>   free(info[i]);
>  }
>  return 0;
> }

> /* This is the one I don't think should work! (though I did when I wrote it)
> */

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

> int main(void)
> {
>         struct dirent *pdirstruct;
>         const char basedir[] = "/usr/home/gregm/public_html";
>         int cnt, i;
>         DIR *dp;
>         char *pdir[FILENAME_MAX];

>         cnt = 0;
>         if((dp = opendir(basedir)) != NULL)
>         {
>                 while((pdirstruct = readdir(dp)))
>                 {
>                         ++cnt;
>                         if((*pdir = realloc(*pdir, sizeof(pdir) *
> cnt))==NULL)

This causes severe undefined behavior.  There are only two valid
things to pass as the first argument to realloc(), NULL or a pointer
which has been allocated by a previous call to malloc(), calloc() or
realloc().  If the pdir array of character pointers had been defined
outside of any function or using the static keyword, all of the
pointers would be NULL.  But they are just uninitialized local data in
a function and passing uninitialized data to any function, or reading
it in any other way with a very few exceptions causes undefined
behavior.

- Show quoted text -

Quote:
>                         {
>                                 puts("realloc() failed");
>                                 exit(0);
>                         }
>                         pdir[cnt] = pdirstruct->d_name;
>                 }
>         }
>         else
>         {
>                 puts("opendir(failed)");
>                 exit(0);
>         }
>         for (i = 0; i < cnt; ++i)
>         {
>                 puts(pdir[i]);
>         }
>         free(*pdir);
>         return 0;
> }

Even if you didn't have the improper use of pointers, that is if you
allocated them with malloc() instead of realloc(), the program
wouldn't do what you wanted.

Each one of the pointers would point to the same member of the same
structure, so each one of them would point to the same string at the
end of the loop, the last one placed in the structure.

You also seem to misunderstand the macro FILENAME_MAX.  It does not
specify the maximum number of files in a directory which can be found
by the POSIX dirent.h functions.  It specifies the maximum length of
the string containing a single filename.

You could indeed use FILENAME_MAX, or any other integral constant
expression, to define the size of your array of pointers to char, but
the you need to stop your loop when have performed that many
iterations.  What happens if there are more than FILENAME_MAX entries
returned by readdir?  Then you are trying to store pointers to char
past the end of the pdir array.

</Jack>
--
Do not email me with questions about programming.
Post them to the appropriate newsgroup.
Followups to my posts are welcome.



Wed, 28 Nov 2001 03:00:00 GMT  
 question about memory allocation
Thanks Jack. A couple comments/questions.

Quote:

>wrote in comp.lang.c:

>>   if((info[cnt-1] = malloc(strlen(someinfo))) == NULL)

>This is a real problem.  When you copy a string the destination must
>contain at least strlen(string) + 1 characters, because the value
>returned by strlen() does NOT count the '\0'.

I had done that because I wanted toreplace the '\n' with a '\0' but I see
that was not a good idea because of the strcpy()

Quote:
>>   {
>>    puts("realloc() failed");
>>    exit(EXIT_FAILURE);
>>   }

>>   strcpy(info[cnt-1], someinfo);

>Since you didn't use strlen()+1 in your malloc(), this copy ends up
>writing a '\0' past the end of the allocated memory.

>>   info[cnt-1][strlen(info[cnt-1])-1] = '\0';
>>  }

>>         char *pdir[FILENAME_MAX];

My intention here had been create an array of pointers to pointers to the
filenames stored in struct dirent, but of course I see as you pointed out I
have gotten it backwards. Does this mean I must declare char **pdir and then
allocate space?

Quote:

>>         cnt = 0;
>>         if((dp = opendir(basedir)) != NULL)
>>         {
>>                 while((pdirstruct = readdir(dp)))
>>                 {
>>                         ++cnt;
>>                         if((*pdir = realloc(*pdir, sizeof(pdir) *
>> cnt))==NULL)

>Even if you didn't have the improper use of pointers, that is if you
>allocated them with malloc() instead of realloc(), the program
>wouldn't do what you wanted.

>Each one of the pointers would point to the same member of the same
>structure, so each one of them would point to the same string at the
>end of the loop, the last one placed in the structure.

Yes I see that now and under windows that's exactly what happened but under
FreeBSD for some mysterious reason the program appeared to work, it's not
the first time my foggy notion of pointers has worked under FreeBSD when it
shouldn't.
Thanks Jack,
Back to the drawing board for me.
Greg.


Wed, 28 Nov 2001 03:00:00 GMT  
 question about memory allocation
Thanks (do I call you raw? :-)

Quote:

>The first program contains no check that cnt doesn't reach MAXSIZE,
>so you may overrun the array info.  You should malloc the string
>length + 1 if you're going to copy a string (the most serious error).
>fgets() might not include the '\n' if the line is too long, which
>should be considered even if not checked.

Yes thankyou - I didn't consider that.

Quote:
>I didn't see any other problems with the first program (I'd be
>thoroughly unsurprised if I missed glaring errors), and laziness
>masquerading as pedantic purity stopped me before I looked at the
>second.

LOL pedantic purity is always is always a good place to hang your hat. By
the way was this language invented by the Marquis de Sade? Either that or
the guy who invented potatoe chips (painful but {*filter*}ive?)
Greg.


Wed, 28 Nov 2001 03:00:00 GMT  
 question about memory allocation

.....snip........

Quote:
> >  char *info[MAXSIZE];
> >  char someinfo[MAXSIZE];

....snip,,,,,

Quote:
> >   ++cnt;
> Any reason not to move the ++cnt to the bottom of the loop and save
> all of the cnt-1 references?

> >   if((info[cnt-1] = malloc(strlen(someinfo))) == NULL)

> >   {
> >    puts("realloc() failed");

Even when your plan is to immediately exit the program, I would suggest that it is
a good memory management practice to always free dynamic allocated memory once you
are finished with it.

Assuming one followed your excellent suggestion of putting the cnt increment at
the end of the loop, I would put here:

while(cnt) free(info[--cnt]);

Quote:
> >    exit(EXIT_FAILURE);
> >   }

--
Al Bowers
Tampa, FL  USA

http://www.gate.net/~abowers/


Thu, 29 Nov 2001 03:00:00 GMT  
 
 [ 6 post ] 

 Relevant Pages 

1. General Question about memory allocation

2. Question about memory allocation

3. Question regarding memory allocation/deallocation

4. few questions about memory allocation

5. Questions on memory allocation and gets()

6. ## question on memory allocation for char strings ##

7. Question on memory allocation

8. Question about memory allocation.

9. question about memory allocation

10. Novice question about memory allocation

11. Tree memory allocation, pool memory allocation

12. URGENT Memory allocation question

 

 
Powered by phpBB® Forum Software