1

C programming newbie here...

I have a function which does some maths and stores the result in an output variable:

void myFunction(char* output) {
   unsigned char myData[64]={0};

   // Call to another function which fills the 'myData' array
   compute(myData);

   // Now let's format the result in our output variable
   for (int n=0; n<64; n++) {
      sprintf(output, "%s%02x", myData[n]);
   }
}

The output char array is allocated by the caller in a variable called result:

void main(void) {
   char* result = NULL;
   result = malloc(129 * sizeof(unsigned char)); // twice the size of myData + 1 ending byte
   myFunction(result);
   // process result here
   // (...)

   free(result);
}

Problem is I always get some garbage stuff at the beginning of result, for instance:

���˶ang/String;8fb5ab60ed2b2c06fa43d[...]

Here the expected data starts at 8fb5ab60ed2b2c06fa43d. After doing some logs, I know that result already contains ���˶ang/String; before the sprintf() loop.

I don't understand how this can occur: isn't the malloc() function supposed to reserve memory for my variable? I guess this garbage comes from another memory area, which will eventually lead to some funky behavior...

That said, I've found a workaround just by adding a null ending byte at the 1st position of result, before calling the function:

    result[0]='\0'; // initialisation
    myFunction(result);

Now it works perfectly but I highly doubt that's good practice... Any advice?

Silas
  • 813
  • 1
  • 10
  • 18
  • 3
    `malloc` doesn't initialize the memory. You're getting ... what was there before. Always initialize your strings like you did. – Jean-François Fabre Apr 16 '18 at 15:39
  • [different context, same principle](https://stackoverflow.com/a/6445794/2371524) ;) –  Apr 16 '18 at 15:40
  • 3
    The problem isn't reading uninitialized bytes, it's appending to `output` incorrectly with invalid format specifiers to `printf`. Reopening. – dbush Apr 16 '18 at 15:46
  • 1
    Save time: enable all compiler warnings - A good compiler will warned about `sprintf(output, "%s%02x", myData[n]);` – chux - Reinstate Monica Apr 16 '18 at 17:17

1 Answers1

4

Here's your problem:

for (int n=0; n<64; n++) {
   sprintf(output, "%s%02x", myData[n]);
}

Your format specifier to sprintf expects a char * followed by an unsigned int. You're only passing in an unsigned char (which is converted to an int), so this character value is being interpreted as an address. Using the wrong format specifier to sprintf invokes undefined behavior.

It looks like you were attempting to append to output. The way to do that would be to only include the %02x format specifier, then on each loop iteration increment the pointer value output by 2 so that the next write happens in the correct place:

for (int n=0; n<64; n++, output+=2) {
   sprintf(output, "%02x", myData[n]);
}
dbush
  • 205,898
  • 23
  • 218
  • 273
  • Indeed, that fixed the problem without the need of initializing the output variable. Thank you for your explanation, things are more clear now :) – Silas Apr 18 '18 at 13:50