2

My source file has a function, which I invoke in the process of de-serializing a struct passed over a C socket (this is the client source):

MY_STRUCT_TYPE myStructVar;

unsigned char * deserialize_chararr(unsigned char *buffer, unsigned char** value)
{
    unsigned char val[256];
    int i;
    for(i = 0; buffer[i]; i++) {
        val[i] = buffer[i];
    }
    val[i++] = '\0';
    printf("\n chararr: %s :", val);

    *value = malloc(i * sizeof **value);
    for(i = 0; buffer[i]; i++) {
        (*value)[i] = buffer[i];
        printf("%c", (*value)[i]);
    }
    (*value)[i++] = '\0';
    /* buffer contains a serialized struct read over a socket using recv. 
so this call returns the buffer pointer that get's passed to the next deserialize_<type> invocation a la https://stackoverflow.com/a/1577174/434145
That's why the first string extraction followed by an attempt to set it in the struct as part of unpacking. */
    return buffer + i;
}

that I'm calling as

buffer = deserialize_chararr(buffer, &myStructVar->szrecordid);

where

typedef struct _MY_STRUCT_TYPE {
    unsigned char   szrecordid[28];
}

The server sends the data alright and it even parses correctly in the above function, but I can't seem to be able to set it in the String variable of my struct. I changed my function to use char ** param after seeing this, but still get rubbish when I print

printf("\n Payload:"  "\n szrecordid: %s ", myStructVar.szrecordid);

In short, I pass a pointer to a string (&myStructVar->szrecordid), use malloc and assign my pointee character by charcter (this latter part I got from the link). Both before and after I changed the code from using a char * to using a char ** I was getting the same error and a compiler warning:

client.c:159: warning: passing arg 2 of `deserialize_chararr' from incompatible pointer type

So what am I doing wrong?

Community
  • 1
  • 1
Nikhil Silveira
  • 504
  • 4
  • 14

3 Answers3

4

The problem is that a pointer to an array is not compatible with a pointer to a pointer. They are different things. And it's a good thing too, because this is meant to catch errors like the one in your code. What you are doing is basically this:

unsigned char szrecordid[28];
szrecordid = malloc(i * sizeof **value);

You should be able to see the problem; you're trying to assign a pointer to an array.

Nikos C.
  • 50,738
  • 9
  • 71
  • 96
1

If I understand correctly, your problem is that you can't change the address of szrecordid:

typedef struct _MY_STRUCT_TYPE {
    unsigned char   szrecordid[28];
}

And if you can't change the address of this variable it's because it's an array. Try using a char * instead or don't malloc and use strcpy instead.

By the way, please consider using strcpy and strdup when you can. Basically, your function is doing:

strcpy(val, buffer);
printf("\n chararr: %s :", val);
*value = strdup(buffer);
printf("%s", value);

And you could also notice that the first strcpy is useless too. I don't understand the meaning of this intermediate char array.

Maxime Chéramy
  • 17,761
  • 8
  • 54
  • 75
  • Point taken on strcpy. strdup won't work though, as I was mistakenly trying to assign a char * to a char []. – Nikhil Silveira Jun 30 '13 at 19:24
  • Marking as answer, though both @Chris & @Nikos were great help in understanding the issue, as I found the answer while writing this comment. So I go back to taking a char * as the formal param, pass the array itself as the arg, to get the first element of the array, and use sprintf to copy into the char array directly. Basically `unsigned char * deserialize_chararr(unsigned char *buffer, unsigned char* value) { return buffer + sprintf(value, buffer) + 1; }` see: http://stackoverflow.com/a/2889483/434145 and http://stackoverflow.com/questions/728535/passing-c-array-as-char-function-parameter – Nikhil Silveira Jun 30 '13 at 19:25
  • Why `sprintf` and not `strcpy`? – Maxime Chéramy Jun 30 '13 at 20:00
  • umm, prob cos my brain stopped working at midnight. You're right strcpy should work too. Though sprintf suits me better as I need to increment the buffer ptr to the next read position. – Nikhil Silveira Jul 01 '13 at 04:43
1

szrecordid is an array of unsigned characters, not a pointer, so when you taks its address (&myStructVar->szrecordid) you get a pointer to an array (an unsigned char (*)[28]), and not a pointer to a pointer, which is what the function expects.

If you think about it, it makes perfect sense -- you can't move an array by simply changing its address, as its address (and size) is fixed at the time it is allocated (when you create whatever it is that myStructVar points at). If you want to change that array, you need to change its contents by copying into into it, and that won't change the size, which is fixed at 28.

The obvious solution to your problem is make it a pointer instead of an array:

typedef struct _MY_STRUCT_TYPE {
    unsigned char   *szrecordid;
}
Chris Dodd
  • 119,907
  • 13
  • 134
  • 226
  • That makes sense. Coupled with @Nikos's answer, I understand the malloc is pointless. But shouldn't the loop where I assign `(*value)[i] = buffer[i];` work, esp if I take out the malloc? Unfortunately that gives me a seg fault. Also, I can't change the struct field to a pointer. That's part of a client api. – Nikhil Silveira Jun 30 '13 at 18:20