Converting old C code

148 Views Asked by At

I have this code snippet in project source code which I work on

void func(void **p,size_t s)
{
    *p = malloc(s+sizeof(size_t));
    *(((size_t *)(*p))++) = s;
}

and gcc-4.7 does not compile it. gcc returns

lvalue required as increment operand 

error message. I changed it into

stp = ((size_t *)(*p));
*(stp ++) = s;

and

stp = ((size_t *)(*p));
*stp = *stp + 1;
*stp = s;

gcc compiles both of them. But application does not work expected. Is conversion true? And is there any tool for conversion?

4

There are 4 best solutions below

0
On BEST ANSWER

The idea seems to be to allocate a certain amount of memory (s) and an additional amount to store this size allocated in the same area as a leading block and then return a pointer to just behind the stored size.

So try this:

void func(void ** p, size_t s)
{
  size_t * sp = malloc(s + sizeof s);
  if (NULL != sp)
  {
    *sp = s;
    ++sp;
  }

  *p = sp;
}

Btw, freeing the allocated memory, is not straight forward.

A typicall sequence of calls, also freeing what this function returns, would look like this then:

void * pv = NULL;
func(&pv, 42);
if (NULL != pv)
{
  /* Use 42 bytes of memory pointed to by pv here. */

  free(((char *) pv) - sizeof (size_t));
}
2
On
*(((size_t *)(*p))++) = s;

Breakdown:
*(
  (
    (size_t *) (*p)
  ) ++
) = s

Means : Take *p as a pointer to size_t (let's call that ptr for after), dereference it (= take the size_t typed value at the address *p), assign that value to s and finally increment ptr (which is to say increment the address in *p by the sizeof(size_t).

You can translate that to:

size_t *ptr = (size_t*)(*p); //second pair of paren is optionnal
s = *ptr;
ptr = ptr + 1; //Warning: This will only modify the variable ptr and not
               //the data at its original place *p, if the remainder of
               //the program is based on that (which I highly suspect)
               //you should do instead :

(size_t*)*p = (size_t*)(*p) + 1; //This also ensures "+1" is treated as
                                 //"add the sizeof(size_t)" because *p
                                 //points to a size_t typed variable

You could also retype a variable and have it point at the same location as p and be done with the casts:

void func(void **p,size_t s)
{
  size_t **ptr = p;
  *ptr = malloc(s+sizeof(size_t));

  if (*ptr == NULL) etc...

  *((*ptr)++) = s;
}
0
On
  • func2() : Adding a variable can increase readability, IMHO. (and inlining will get rid of it, afterwards)
  • The second function func3() demonstrates that using the return value instead of passing a (opaque) pointer by reference can avoid complexity and casts

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

void func2(void **p,size_t s)
{
    size_t **pp;
    pp = (size_t **) p; // only one cast
    *pp = malloc(s+sizeof(size_t));
    if (!*pp) return;
                          fprintf(stderr,"[mallocd %p]", *pp);
    *pp[0] = s;
    *pp += 1;
}

void *func3(size_t s)
{
    size_t *p;
    p = malloc(s+sizeof *p);
    if (!p) return NULL;
                          fprintf(stderr,"[mallocd %p]", p);
    p[0] = s;
    return p+1;
}

int main(void)
{
char *p = NULL , *p2 = NULL;

func2( (void**) &p, 666);       // yet another cast
fprintf(stderr,"returned p := %p\n", p);

p2 = func3( 666); // No cast!!!
fprintf(stderr,"returned p2 := %p\n", p2);

return 0;
}
0
On

try

void func(void **p,size_t s)
{
    *p = malloc(s+sizeof(size_t));
    *(*(size_t **)p)++ = s;
}

This is allocating s bytes of memory, plus enough extra to hold s, storing s at the start of that space and modifying *p to point just after that.

More sensible and clearer (and avoiding casts!) would be:

void *func(size_t s)
{
    size_t *p = malloc(s + sizeof(size_t));
    if (p) *p++ = s;
    return p;
}

but that requires changing the code that calls this function to take the return value and store it wherever desired, rather than passing an extra pointer as an argument:

some_ptr = func(needed_size);

rather than

func((void **)&some_ptr, needed_size);

also avoiding casts...