[vox-tech] Must one free() in C?

vox-tech@lists.lugod.org vox-tech@lists.lugod.org
Wed, 8 May 2002 20:08:14 -0400


Tim, Mark,

  For what it's worth in most functions I write I tend to do the
following sort of flow, since most code is a series of operations that 
if anything failed you have to stop, and possibly clean up after 
yourself...

  The following flow makes cleanup very easy, following what is happing 
easy (after you get used to the structure), and debugging becomes *trivial* 
(if you do this everywhere, anything going wrong gives you a cascade
stack dump of what happened and who it affected).

  I have considered looking into varargs or whatever so that the fprintf
can be changed to...
  fiz_error("blap zap %s %d %f\n", a_string, an_int, a_float);
which would make the cascaded if much easier to read.  But haven't yet.


int some_init()
{
  int *data1 = 0;                 /* initialize everything to empty */
  float *data2 = 0;
  int file = -1;
  int result = 1;                 /* some error case */

  if (!(data1 == malloc(sizeof(data1) * MAGIC_NUMBER1)))
    fprintf(stderr, "%s:%d %s error: "
            "couldn't allocate room for foobar (%d)\n",
            __FILE__, __LINE__, __FUNCTION__, sizeof(data1) * MAGIC_NUMBER1);
  else if (!(data1 == malloc(sizeof(data2) * MAGIC_NUMBER1)))
    fprintf(stderr, "%s:%d %s error: "
            "couldn't allocate room for bazzip (%d)\n",
            __FILE__, __LINE__, __FUNCTION__, sizeof(data2) * MAGIC_NUMBER1);
  else if ((file = open(file_name, O_RDONLY)))
    fprintf(stderr, "%s:%d %s error: failed to open \"%s\", %d : %s\n",
            __FILE__, __LINE__, __FUNCTION__,
            filename, errno, strerror(errno));
  else if (result = magic_loader(file, data1, data2))
    fprintf(stderr, "%s:%d %s error: magic_loader failed %d\n",
            __FILE__, __LINE__, __FUNCTION__, result);
  else
  { /* all setup worked */
    result = play_with_data(data1, data2);
  }

  /* cleanup */
  free(data1);
  free(data2);
  if (file != -1)
    close(file);

  return result;
}

  TTFN,
    Mike

pps:

  free(0) is valid and defined as a no-op.  some annoying memory 
manipulation checker programs may complain about those, but the complaint 
can usually be turned off and if it can't then just change to
"if (foo) free(foo);"

ps:
  I have also gotten a complaint that I shouldn't send things to stderr
the code of internal functions, but return a unique error code which the
caller can then lookup in some lookup table to determine the error reason
and message it wants to print.  In practice I think this is much more 
useful, because it insures that errors are reported without implementing 
the error reporting in every caller, the errors reported are always "should
never happen" type errors, the worker function has complete access to all 
useful data variables to print an appropriate full detail message, and
if for some reason one wants error crap turned off the coder can simply
close stderr.  *=>


On Wed, May 08, 2002 at 11:59:12AM -0700, Tim Riley wrote:
> If this program were ported to DOS/Windows, I would rewrite it as:
> 
> if  ( ! ( tmp1 = (int *)malloc(5) ) )
> {
>     fprintf( stderr, "Out of memory\n" );
>     exit( 1 );
> }
> 
> if ( ! ( tmp2 = (int *)malloc(6) ) )
> {
>     fprintf( stderr, "Out of memory\n" );
>     free( tmp1 );
>     exit( 1 );
> }
> 
> "Mark K. Kim" wrote:
> > Let's say you got a code like this (in C):
> >
> >    tmp1 = (int*)malloc(5);
> >    tmp2 = (int*)malloc(6);
> >    if(!tmp1 || !tmp2)
> >    {
> >       fprintf(stderr, "Out of memory!\n");
> >       exit(1);
> >    }