4

So I have a method that returns an unsigned char *

unsigned char* someMethod(num)
unsigned short num;
{
    //do some stuff with num and change values of a
    unsigned char * a = (unsigned char*) malloc(4);
    printf("a0 is %x\n",a[0]);
    printf("a1 is %x\n",a[1]);
    printf("a2 is %x\n",a[2]);
    printf("a3 is %x\n",a[3]);
    return a;
}

When I called someMethod(128):

unsigned char* s = someMethod(128);
printf("s0 is %x\n",s[0]);
printf("s1 is %x\n",s[1]);
printf("s2 is %x\n",s[2]);
printf("s3 is %x\n",s[3]);

It would print out

a0 is 30
a1 is 1
a2 is 31
a3 is 30
s0 is 30
s1 is 14
s2 is ffffff9d
s3 is 0

It makes no sense to me at all since I assign s = someMethod(128). Shouldn't a and s have the same values?!? Any help is appreciated. Thank you!

Dao Lam
  • 2,837
  • 11
  • 38
  • 44
  • 8
    A K&R style function definition? How old is that code? – Daniel Fischer Nov 30 '12 at 01:15
  • 4
    I suspect you haven't `#include`d `stdlib.h`, so the return type is assumed to be `int`. Apart from that, `%x` is the wrong format for `unsigned char` (though in this case, it would probably work). – Daniel Fischer Nov 30 '12 at 01:19
  • 2
    Can you reproduce the problem with the function as posted? (The function looks simplified, which might miss the actual problem) – sth Nov 30 '12 at 01:21
  • 1
    If your problem is that you are missing a prototype for `malloc()` as Daniel Fischer suggests, let that be a lesson about casting the result of `malloc()` (In C, you shouldn't). – Pascal Cuoq Nov 30 '12 at 01:24
  • Even though you're using unsigned char, I recommend you use the sizeof operator on the data type to make sure data is being properly allocated: unsigned char * a = (unsigned char*) malloc(sizeof(unsigned char)*4); – Gustavo Litovsky Nov 30 '12 at 01:51
  • The portions of code you have presented are unlikely to produce the results you report. You should present a [short, self-contained, compilable example](http://sscce.org/). It is theoretically possible that failing to include `` (which declares `malloc`) could lead to this behavior, but it is unlikely in common platforms. It is likely there is something else in your code interfering. – Eric Postpischil Nov 30 '12 at 03:13
  • What compiler and platform is this? – Russell Borogove Nov 30 '12 at 04:24
  • 1
    @gl3829: Better: `unsigned char *a = malloc(4 * sizeof *a);`. The cast is unnecessary (the `void*` result is implicitly converted to the appropriate type) and can mask errors in some cases. Using `sizeof *a` rather than `sizeof (unsigned char)` makes the code more maintainable; you don't need to change two things if the type of `a` changes. – Keith Thompson Nov 30 '12 at 21:51
  • I didn't know you could do it that way. If it was an array I would have used the sizeof of the first element. This would be the same as what you're suggesting. – Gustavo Litovsky Nov 30 '12 at 22:20

2 Answers2

1

In reference to @gl3829's comment, I'd go with

unsigned char *a = malloc(4 * sizeof(*a))

so that the size is "automatically" correct.

More importantly, I think a problem is that, in someMethod, you print out the values in the allocated array before actually assigning anything. This invokes undefined behavior and is allowed to yield any results. Try storing something before printing them out.

For printing unsigned chars in hexadecimal, the correct format specifier is %hhx. Using the wrong specifier can also invoke undefined behavior.

Joshua Green
  • 1,517
  • 1
  • 10
  • 14
  • The behavior of this code is not undefined. Per C 2011 7.22.3.4 2, `malloc` returns an object with indeterminate value. And `unsigned char` is special. Per 6.2.6.2 1, it has a binary representation (and hence no trap representations) and no padding bits. Passing an `unsigned char` allocated with malloc to `printf` with an appropriate specifier **must** print some value. – Eric Postpischil Nov 30 '12 at 03:01
  • The specifier is not a problem. The `unsigned char` that is passed to `printf` is converted to `int` before being passed, and printing it with `%x` produces the same result as printing it with `%hhx`. – Eric Postpischil Nov 30 '12 at 03:08
  • 1
    Interesting. I grabbed my thoughts about initialization from [here](http://stackoverflow.com/questions/367633/what-are-all-the-common-undefined-behaviour-that-a-c-programmer-should-know-ab), specifically "Dereferencing a pointer that has not yet been definitely initialized" -- I guess `malloc` does something a bit special. As for `printf`, using the wrong specifier _does_ invoke undefined behavior, e.g. [here](http://stackoverflow.com/questions/4231891/why-does-using-the-wrong-format-specifier-in-c-crash-my-program-on-windows-7), but it's not clear if the length specifier `hh` is necessary. – Joshua Green Nov 30 '12 at 03:26
  • Of course, by "not clear" I mean "not clear _to me_." I'd be quite happy to be enlightened. – Joshua Green Nov 30 '12 at 03:33
  • “Dereferencing a pointer that has not yet been definitely initialized” means `char *p; *p;`—It is `p` that has not been initialized, so evaluating `*p` attempts to use `p`. In this case, the pointer `a` is initialized by assigning it the value of `malloc`; it is the *contents* of `a` that are not initialized. – Eric Postpischil Nov 30 '12 at 03:34
  • I wonder why dereferencing an uninitialized pointer should be "worse" than using an uninitialized value in some other way. I always thought they were both undefined, though I now vaguely recall some strange exception for specific classes of `char`s. – Joshua Green Nov 30 '12 at 03:41
  • De-referencing uninitialized pointers may lead to segmentation fault. – billyswong Nov 30 '12 at 04:11
1

When you are printing in %x, the printf() function reads an int. An int in 32-bit computers is 4 bytes long. Therefore, the 2nd, 3rd and 4th printf() are reading both outside and inside the malloc() area. You can't expect areas outside what you malloc'd always stay unchanged.

The solution is, pad it. Malloc a few bytes more, probably sizeof(int) more, than what you currently want.


Solution 2: Type cast those s[i] to int first before passing it to printf()

billyswong
  • 1,114
  • 7
  • 12
  • 1
    The expressions passed to `printf` are `a[0]`, `a[1]`, and so on. Since `a` is a pointer to `unsigned char`, these are `unsigned char` objects, and they are within the allocated array. They are converted to `int` before being passed to `printf`, and the array is not overrun. – Eric Postpischil Nov 30 '12 at 03:06
  • Normally function parameters do convert itself to their intended types before feeding to the function. However, `printf()` function could NOT specify what types of variables it is going to receive in its function declaration, because those parameter types depend on the format string, a parameter itself. – billyswong Nov 30 '12 at 03:25
  • 1
    For functions declared with an ellipsis, such as `printf`, the default argument promotions are performed on the corresponding arguments, per C 2011 6.5.2.2 7. The default argument promotions include the integer promotions, per 6.5.2.2 6, and the integer promotions convert `unsigned char` to `int` per 6.3.1.1 2 (except, in certain perverse C implementations, `unsigned char` can be promoted to `unsigned int`). – Eric Postpischil Nov 30 '12 at 03:40
  • 1
    In any case, the address of `a` is not passed to `printf`, so `printf` has no way of locating anything in `a` or overrunning the space. `printf` only receives the values passed to it, not the addresses of those values. – Eric Postpischil Nov 30 '12 at 03:41