manipulation and memory allocation

[Flash Gordon]

32? Do you mean sizeof(n)*CHAR_BIT ?
Agreed.



This sequence looks functionally equivalent to:

s = (char *) realloc (s, (b + 1) * sizeof (char));

No, the realloc call has to copy the data if it moves where the block is
whilst freeing then allocating a new block does not. Not too important
for a small buffer, but I sometimes work with large buffers where it
could matter.

I would not bother testing if s was NULL though.

However, you don't seem to have done anything about the case of
receiving a NULL from malloc (if/when you run out of memory).
True.

for(i = b - 1; i > -1; i--) {
if((0x01 << i) & n) {
s[b - i - 1] = '1';
} else {
s[b - i - 1] = '0';
}
}
s = '\0';
return s;
}

This looks like its technically correct but a lot more complicated than
it needs to be. How about:

for (i = b - 1; i >= 0; i--)
s = ((1 << (b-i-1)) & n) ? '1' : '0';

I'm not sure it is technically correct. I think you hit undefined
behaviour when you shift the 1 in to the signed bit. Although I've not
used systems where it would fail.

Or even:

for (i = b - 1; i >= 0; i--)
s = '0' + ((1 << (b-i-1)) & n);

Correct me if I'm wrong, but won't ((1 << (b-i-1)) & n) give numbers
other than 0 and 1? Which just goes to show what you think of as less
complicated isn't always. I consider the code you were responding to
just as simple and it avoids the bug you just introduced.

Well, drop the "int" before the b and you are set. BTW, your function
actually is strangely compatible with this already -- but you can see
the advantage of the "realloc" simplification.

I think it would be simpler to either not pass in a pointer to a buffer
or to specify that the buffer had to be large enough.

Well, not checking the return of malloc is bad. Otherwise this all
depends on your intended semantics. The function will allocate memory
(if you pass it NULL) for a NUL terminated string containing a textual
representation of the binary bits of a 2s complement number and will
recycle allocated space if you call it repeatedly (which makes the
advantage of using realloc clear.) Is this how you are intending to
use it? You might rename it as "to_bits_realloc()" to be explicit
about its semantics, and there are clearly ways of speeding this up
using tables.

But other than that, the code at least looks correct.

I would say that always calling realloc on the buffer was a bad idea for
many reasons. Including it preventing you from passing a char array to
the routine.