Problem in printing string, created in another function.

D

DanielJohnson

I am trying to print a int8_t (8 bits) in binary. Following is my
code. While I can print the values successfully within the function
call but when I try to print the same in main, garbage gets printed.
This makes me feel that since I assign retval memory using automatic
memory allocation in print8bits, after the function exits, that memory
is deallocated. Should I actually use malloc/calloc to assign memory
to retval so that even after function exits the value is still there.
Please advise.

Is there a better way to write and return values in such scenarios
where a function allocates the memory and the main just prints it.

Thanks for your replies.


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

#define CHAR_BIT 8

char *print8bits(int8_t a){
int8_t mask = 1;
char retval[] = "00000000\0";
int i, j = sizeof(a)* CHAR_BIT -1;
for(i = 0; i < sizeof(int8_t) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 8 bit integer = %s, %d\n", retval, a); //prints
fine
printf("Address of retval = %p\n", retval); //Address are same in
both the places
return retval;
}

int main()
{
char *temp = print8bits(-15); // I want to print -15 as 11110001.
printf("Address of retval in main = %p\n", temp); //Address are
same in both the places
printf("Printing 8 bit integer: %s\n", temp); // prints garbage
return 0;
}
 
E

Eric Sosman

DanielJohnson said:
I am trying to print a int8_t (8 bits) in binary. Following is my
code. While I can print the values successfully within the function
call but when I try to print the same in main, garbage gets printed.
This makes me feel that since I assign retval memory using automatic
memory allocation in print8bits, after the function exits, that memory
is deallocated. [...]

You're on the trail of the problem. See Question 7.5a in
the comp.lang.c Frequently Asked Questions (FAQ) list at
<http://www.c-faq.com/>.
 
D

DanielJohnson

I forgot to add that the compiler prints the following warning when I
compiled it using gcc 4.2.

print_bytes_in_binary.c: In function ‘print8bits’:
print_bytes_in_binary.c:18: warning: function returns address of local
variable

Sorry for not providing complete information in my first post.

Thanks.
 
J

JC

DanielJohnson said:
I am trying to print a int8_t (8 bits) in binary. Following is my
code. While I can print the values successfully within the function
call but when I try to print the same in main, garbage gets printed.
This makes me feel that since I assign retval memory using automatic
memory allocation in print8bits, after the function exits, that memory
is deallocated. [...]

     You're on the trail of the problem.  See Question 7.5a in
the comp.lang.c Frequently Asked Questions (FAQ) list at
<http://www.c-faq.com/>.

@DanielJohnson:

A possible approach is to have the caller supply the buffer; you can
still return a pointer to the buffer if you want to be able to call
the function in line:


char * print8bits (int8_t a, char *dest, int destbytes) {
/* convert and store in dest, do not write more that destbytes */
return dest;
}


Then the usage is:


int main (void) {
char temp[9];
printf("it's %s\n", print8bits(-15, temp, sizeof(temp)));
printf("it's still %s\n", temp);
return 0;
}


Note that passing the length of the buffer to the function lets the
function stop if it's going to write past the end of the buffer, but
if you know that your function will never write more than a certain
number of bytes, you could leave the length out and simply require
that the supplied buffer be at least 9 bytes long (or whatever). It's
usually a good idea to pass the buffer length, though, you never know
how things will change in the future.


Jason
 
D

DanielJohnson

     You're on the trail of the problem.  See Question 7.5a in
the comp.lang.c Frequently Asked Questions (FAQ) list at
<http://www.c-faq.com/>.

Thanks for quick pointer to the right place and my bad that I did not
look it up before posting.

By using static retval and malloc(), I can overcome the problem.

A quick question, how can I initialize value using malloc(). For
example, following is not valid.

char *retval = malloc(CHAR_BIT + 1);
retval = "00000000\0";

Is there a terse way of doing it ? I know I could do in a for loop by
assigning retval a value of '0' each time. I wanted to learn a
smarter way.

Thanks.
 
D

dj3vande

I am trying to print a int8_t (8 bits) in binary. Following is my
code. While I can print the values successfully within the function
call but when I try to print the same in main, garbage gets printed.
This makes me feel that since I assign retval memory using automatic
memory allocation in print8bits, after the function exits, that memory
is deallocated.
Exactly.

Should I actually use malloc/calloc to assign memory
to retval so that even after function exits the value is still there.
Please advise.

That's one way to solve the problem.
If you do that, the caller will need to free the memory after it's
finished using it.

Is there a better way to write and return values in such scenarios
where a function allocates the memory and the main just prints it.

Other possible solutions:

(1) Make the array you format into static.
This imposes some nontrivial restrictions on how the returned pointer
is used, but makes things most convenient for the caller when those
restrictions are satisfied.
The caller MUST completely finish using the buffer you formatted the
string into before the next time the formatting routine is called,
because the previous value will be overwritten every time it gets
called.
Also, it will be Very Difficult to make this thread-safe, if that's
something you're concerned about. (Both the call and the use of the
returned buffer need to be serialized, so you can't do it inside the
formatting code.)

(2) Require the caller to give you a buffer to format into.
This is probably the one I'd use.
It makes things a little bit less convenient for the caller (you need
to allocate a working buffer there, even if all you're going to do is
give the string to printf), but it deals with the storage-duration
problems that automatic or static allocation in the formatting code
have, and leaving memory management entirely up to the caller makes
reading the code easier, since it's all in one place. (It's also
likely that the caller knows better than the library code what the best
way to allocate the memory is.)
If the caller is providing the buffer, you also need to make sure the
buffer is big enough for what you're putting into it. For fixed sizes,
documenting it as "needs to provide a buffer at least N bytes long" is
fine; otherwise, take a length argument and return an error if you
don't have enough space.



[Code with all but the relevant bits snipped:]
char *print8bits(int8_t a){ [...]
char retval[] = "00000000\0"; [...]
return retval;
}


dave
 
D

dj3vande

DanielJohnson said:
A quick question, how can I initialize value using malloc(). For
example, following is not valid.

char *retval = malloc(CHAR_BIT + 1);
retval = "00000000\0";

Is there a terse way of doing it ? I know I could do in a for loop by
assigning retval a value of '0' each time. I wanted to learn a
smarter way.


strcpy().


dave
 
S

Stephen Sprunk

DanielJohnson said:
I forgot to add that the compiler prints the following warning when I
compiled it using gcc 4.2.

print_bytes_in_binary.c: In function ‘print8bits’:
print_bytes_in_binary.c:18: warning: function returns address of local
variable

This is exactly what your problem is; be thankful that your compiler
warned you, since it isn't required to.

In short, your "retval" variable ceases to exist when the function
returns. When you later tell printf() to access that variable, you will
get undefined behavior (e.g. printing garbage). There are numerous ways
to fix the problem, but the first step is recognizing what's wrong :)
Sorry for not providing complete information in my first post.

The error was obvious to most of us without it, fortunately. You
provided (almost) compilable code that exhibits the problem, which is
better than many folks do when asking for help...

S
 
E

Eric Sosman

DanielJohnson said:
You're on the trail of the problem. See Question 7.5a in
the comp.lang.c Frequently Asked Questions (FAQ) list at
<http://www.c-faq.com/>.

Thanks for quick pointer to the right place and my bad that I did not
look it up before posting.

By using static retval and malloc(), I can overcome the problem.

A quick question, how can I initialize value using malloc(). For
example, following is not valid.

char *retval = malloc(CHAR_BIT + 1);
retval = "00000000\0";

Is there a terse way of doing it ? I know I could do in a for loop by
assigning retval a value of '0' each time. I wanted to learn a
smarter way.


The character most easily overlooked is the rightmost,
where you must deposit a '\0'. In your code, this would be
most easily done by changing the calculation of `j' and
adding one line:

int i, j = sizeof(a)* CHAR_BIT; /* -1 removed */
retval[j--] = '\0'; /* added */

For the remaining digits, you could add an `else' to
your existing `if':

if (a & mask)
retval[j] = '1';
else /* added */
retval[j] = '0'; /* added */

Alternatively, you could use the ternary operator:

retval[j] = (a & mask) ? '1' : '0';

A slightly briefer and trickier way would be:

retval[j] = '1' - !(a & mask);

Finally, you might consider getting rid of `mask':
instead of shifting it successively to the left, consider
shifting `a' rightward so the bits slide successively into
the ones' place. Then you could write

retval[j] = '0' + (a & 1);

This last suggestion has a flaw: right-shifting a negative
value gives an implementation-defined result. But you've
already chosen to ignore what happens when you try to store
the value 128 in `mask' (whose maximum is only 127), so I
suspect you'd be willing to take your chances on the shift,
too.
 
D

DanielJohnson

     Alternatively, you could use the ternary operator:
        retval[j] = (a & mask) ? '1' : '0';

     A slightly briefer and trickier way would be:

        retval[j] = '1' - !(a & mask);

     Finally, you might consider getting rid of `mask':
instead of shifting it successively to the left, consider
shifting `a' rightward so the bits slide successively into
the ones' place.  Then you could write

        retval[j] = '0' + (a & 1);

This last suggestion has a flaw: right-shifting a negative
value gives an implementation-defined result.  But you've
already chosen to ignore what happens when you try to store
the value 128 in `mask' (whose maximum is only 127), so I
suspect you'd be willing to take your chances on the shift,
too.

Thanks for all the cool tips and suggestions. Since you mentioned if I
was interested only in the values up to 2^n-1, this is what I was
trying to do.I was trying to understand the bigger problem, i.e., how
different architectures interpret the bytes internally. I was trying
to understand how a 32 bit float stores a number say 3.1422357234
internally in its 32 bits, and thats why I wrote this program. I
wanted to understand how positive and negative (2s complement) are
stored internally. Following is my code.

I hate to post this much amount of code but I want to get suggestions
on how to write better code, and if there are any obvious mistakes in
the programs that would give me incorrect results and hence my
understanding about these issues wouldn't be correct.


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

#define CHAR_BIT 8

void print8bits(int8_t a){
int8_t mask = 1;
char retval[] = "00000000\0";
int i, j = sizeof(a)* CHAR_BIT -1;
for(i = 0; i < sizeof(int8_t) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 8 bit integer = %s, %d\n", retval, a);
}

char *print8bitsW(int8_t a){
int8_t mask = 1;
char *retval = malloc(9);
memcpy(retval, "00000000\0", strlen("00000000\0"));
int i, j = sizeof(a)* CHAR_BIT -1;
for(i = 0; i < sizeof(int8_t) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 8 bit integer = %s, %d\n", retval, a);
return retval;
}
void print16bits(int16_t a){
int16_t mask = 1;
char retval[] = "0000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 16 bit integer = %s, %d\n", retval, a);
}

void print32bits(int32_t a){
int32_t mask = 1;
char retval[] = "00000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 32 bit integer = %s, %d\n", retval, a);
}

void printfloat(float a){
int32_t mask = 1;
char retval[] = "00000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;
uint32_t *p = (uint32_t *) &a;
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (*p & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 32 bit float = %s, %f\n", retval, a);
}

void printdouble(double a){
int32_t mask;
char retval[] =
"0000000000000000000000000000000000000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1, k;
uint32_t *p = (uint32_t *) &a;
/*
* p now points to the last 32 bit of the 96 bits. After each
* iteration we consider the lower 32 bits. For example, in first
* iteration we consider bbits from 95 to 64, then 63 to 32 and
* finally 31 to 0.
*/
p += (sizeof(a)/sizeof(mask))-1;
for (k = 0; k < (sizeof(a)/sizeof(mask)); k++){
p = p - k;
mask = 1;
for(i = 0; i < sizeof(mask) * CHAR_BIT; i++)
{
if (*p & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
}

printf("Printing 64 bit doble = %s, %lf\n", retval, a);
}
void printlongdouble(long double a){
int32_t mask;
char retval[] =
"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1, k;
uint32_t *p = (uint32_t *) &a;
/*
* p now points to the last 32 bit of the 96 bits. After each
* iteration we consider the lower 32 bits. For example, in first
* iteration we consider bbits from 95 to 64, then 63 to 32 and
* finally 31 to 0.
*/
p += (sizeof(a)/sizeof(mask))-1;
for (k = 0; k < (sizeof(a)/sizeof(mask)); k++){
p = p - k;
mask = 1;
for(i = 0; i < sizeof(mask) * CHAR_BIT; i++)
{
if (*p & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
}

printf("Printing 96 bit long doble = %s, %Lf\n", retval, a);
}

void printlong(long a){
long mask = 1;
char retval[] = "00000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 32 bit long = %s, %ld\n", retval, a);
}

void printlonglong(long long a){
long long mask = 1;
char retval[] =
"0000000000000000000000000000000000000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 64 bit long long = %s, %lld\n", retval, a);
}
int main()
{
print8bits(5);
print8bits(-5);
print16bits(123);
print16bits(-123);
print32bits(27345234);
print32bits(-27345234);
printfloat(3.14);
printfloat(-3.14);
printfloat(1265654.3414);
printfloat(-1265654.3414);
printlong(1234126534);
printlong(-1234126534);
printdouble(423432.42342342);
printdouble(-423432.42342342);
printlonglong(-65656361);
printlonglong(65656361);
printlongdouble(-1765347347537.234234234);
printlongdouble(1765347347537.234234234);
return 0;
}

I don't expect you to browse the whole code but if you could point out
any obvious mistakes, that would be great.

Thanks to all of you for your replies.

Daniel
 
B

Ben Bacarisse

Thanks for all the cool tips and suggestions. Since you mentioned if I
was interested only in the values up to 2^n-1, this is what I was
trying to do.I was trying to understand the bigger problem, i.e., how
different architectures interpret the bytes internally. I was trying
to understand how a 32 bit float stores a number say 3.1422357234
internally in its 32 bits, and thats why I wrote this program. I
wanted to understand how positive and negative (2s complement) are
stored internally. Following is my code.

I hate to post this much amount of code but I want to get suggestions
on how to write better code, and if there are any obvious mistakes in
the programs that would give me incorrect results and hence my
understanding about these issues wouldn't be correct.

To look at the big picture for first, my main complaint would be that
you are not using functions enough. You have a lot of duplicated
code. This is probably because you have different types and you can't
see how to write a general "bit-printer".

The key to this is that C allows all objects to be treated as a array
of characters (unsigned char is best). A pointer to any object can be
converted to void *, so all we need is a general function:

void bprint(const void *, size_t size);

that converts the void * to a const unsigned char * and prints the
bits in each of these. You need the size because you just can't tell
when to stop otherwise. Of course, function decomposition means that
I need a function:

void bprint_uchar(unsigned char c);

This will be very similar to what you have already (but see below for
why unsigned char helps rather than using int8_t). A simple loop is
all that is needed to write bprint using bprint_uchar.

All you other functions then become one-liners:

void bprint_int(int a) { bprint(&a, sizeof a); }
void bprint_float(float a) { bprint(&a, sizeof a); }
void bprint_fouble(double a) { bprint(&a, sizeof a); }

A few observations on your code:

void print8bits(int8_t a){
int8_t mask = 1;
char retval[] = "00000000\0";

This makes retval an array of 10 bytes with two '\0's as the end. You
don't need the explicit \0 and it may even confuse some people.
int i, j = sizeof(a)* CHAR_BIT -1;
for(i = 0; i < sizeof(int8_t) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;

This can lead to undefined behaviour. Because mask is a signed int
type you can get odd results for some shifts. Rather than quote all
the rules (they are in the standard if you are interested) it is
simply best to do bit operations only on unsigned types.

This is why I'd start with a function to print the bits in an unsigned
char. The code will be less likely to run into undefined- or
implementation-defined behaviour.
j--;
}
printf("Printing 8 bit integer = %s, %d\n", retval, a);

If all you are doing is printing the result, I would not bother to put
it in a string -- I'd print in the loop doing it from the "top" big
down.
}

char *print8bitsW(int8_t a){
int8_t mask = 1;
char *retval = malloc(9);
memcpy(retval, "00000000\0", strlen("00000000\0"));

This does not do what you want. The strlen returns 8, yes, 8. The
result is that retval gets 8 '0's put into it but no terminating null
despite you having two of them in the strings you use! All you need
is:

strcpy(retval, "00000000");

void print32bits(int32_t a){
int32_t mask = 1;
char retval[] = "00000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;

It seems a shame after all the care of using sizeof that you have to
have a literal string of exactly the right size. You can write:

char retval[sizeof a * CHAR_BIT + 1];
memset(retval, '0', sizeof a * CHAR_BIT);
retval[sizeof a * CHAR_BIT] = '\0';

but overall I would organise the code differently as I explained up
top.
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 32 bit integer = %s, %d\n", retval, a);
}

void printfloat(float a){
int32_t mask = 1;

Here you have the problem that int32_t may not be "wide" enough to
mask all the bits in a float. That almost certainly happens with
double.
char retval[] = "00000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;
uint32_t *p = (uint32_t *) &a;
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (*p & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 32 bit float = %s, %f\n", retval, a);
}

void printdouble(double a){
int32_t mask;

Yup. It is possible but unlikely that double is only 32 bits wide.

<snip remaining code>
 
D

dj3vande

DanielJohnson said:
Thanks for all the cool tips and suggestions. Since you mentioned if I
was interested only in the values up to 2^n-1, this is what I was
trying to do.I was trying to understand the bigger problem, i.e., how
different architectures interpret the bytes internally.

For bonus enlightenment points, go looking for the weirdest systems you
can find, and run your code on them.
Modern desktop hardware is a lot more homogenous than what you'll find
if you look a few decades back and/or beyond the desktop.
I was trying
to understand how a 32 bit float stores a number say 3.1422357234
internally in its 32 bits, and thats why I wrote this program. I
wanted to understand how positive and negative (2s complement) are
stored internally. Following is my code.

I think you're working too hard.
If examining representations is what you're interested in, you can save
yourself some work by writing a single "walk through N bytes and dump
them" function, and calling that directly for the things you want to
examine the representations of.

Something like this:
--------
#include <stdio.h>

/*For demonstration purposes, just dump in hex to stdout.
To mirror your code, you would want to change printbyte to do binary,
and for uses other than poking around at representations for
amusement/enlightenment, formatting into a buffer instead of dumping
to stdout might be useful.
*/
void printbyte(unsigned char c)
{
/*If CHAR_BIT > 8 and you want nicely formatted output, the format
specifier may need some massaging.
*/
printf("%02x ",(unsigned)c);
}

void printbytes(void *v,size_t sz)
{
unsigned char *b=v;
while(sz)
{
printbyte(*b);
b++; sz--;
}
}

int main(void)
{
signed char c;
short s;
long l;
float f;
double d;

printf("Printing char (+/- 5):\n");
c=5;
printbytes(&c,sizeof c); putchar('\n');
c=-5;
printbytes(&c,sizeof c); putchar('\n');

printf("Printing short (+/- 123):\n");
s=123;
printbytes(&s,sizeof s); putchar('\n');
s=-123;
printbytes(&s,sizeof s); putchar('\n');

printf("Printing long (+/- 27345235):\n");
l=27345235;
printbytes(&l,sizeof l); putchar('\n');
l=-27345235;
printbytes(&l,sizeof l); putchar('\n');

printf("Printing float (+/- 3.14):\n");
f=3.14;
printbytes(&f,sizeof f); putchar('\n');
f=-3.14;
printbytes(&f,sizeof f); putchar('\n');

printf("Printing double (+/- 423432.42342342):\n");
d=423432.42342342;
printbytes(&d,sizeof d); putchar('\n');
d=-423432.42342342;
printbytes(&d,sizeof d); putchar('\n');

return 0;
}
--------
[...] I want to get suggestions
on how to write better code, and if there are any obvious mistakes in
the programs that would give me incorrect results and hence my
understanding about these issues wouldn't be correct.

int8_t mask = 1;

Not all newsreaders display tabs properly; expanding them to spaces
before you post will make your code more readable for people with such
(broken) newsreaders.

memcpy(retval, "00000000\0", strlen("00000000\0"));

This will NOT copy the terminating '\0', and you probably want that as
well.
strcpy(retval,"00000000");
will do what I'm pretty sure you wanted to do here: Copy up to and
including the '\0' that terminates the string.

void printfloat(float a){
int32_t mask = 1;
char retval[] = "00000000000000000000000000000000\0";
int i, j = sizeof(a) * CHAR_BIT - 1;
uint32_t *p = (uint32_t *) &a;
for(i = 0; i < sizeof(a) * CHAR_BIT; i++)
{
if (*p & mask)
retval[j] = '1';
mask = mask << 1;
j--;
}
printf("Printing 32 bit float = %s, %f\n", retval, a);
}

Note that something like this only works if float and int32_t (or, more
generally, the two types you're substituting for each other) are the
same size, which isn't guaranteed.

void printdouble(double a){ [...]
uint32_t *p = (uint32_t *) &a;
/*
* p now points to the last 32 bit of the 96 bits. After each
* iteration we consider the lower 32 bits. For example, in first
* iteration we consider bbits from 95 to 64, then 63 to 32 and
* finally 31 to 0.
*/
p += (sizeof(a)/sizeof(mask))-1;
for (k = 0; k < (sizeof(a)/sizeof(mask)); k++){
p = p - k;

I think you're pushing p too far back here.
You probably want either
p=(uint32_t *)&a + (sizeof a / sizeof mask) - (1+k)
to reposition p to the kth-last word, or (rather cleaner in this case)
p--;
to move p one word closer to the beginning.

Note also that looking at larger objects in int32_t-sized chunks only
works if their size is a multiple of int32_t's. Using unsigned char
(whose size is 1 byte by definition) avoids this problem.



dave

--
Dave Vandervies dj3vande at eskimo dot com
Genghis Khan, Immanuel Kant.
In today's lesson, we learn that brute force is more productive than thinking.
--Chris Suslowicz and David P. Murphy in the scary devil monastery
 
B

Barry Schwarz

I am trying to print a int8_t (8 bits) in binary. Following is my
code. While I can print the values successfully within the function
call but when I try to print the same in main, garbage gets printed.
This makes me feel that since I assign retval memory using automatic
memory allocation in print8bits, after the function exits, that memory
is deallocated. Should I actually use malloc/calloc to assign memory
to retval so that even after function exits the value is still there.
Please advise.

Is there a better way to write and return values in such scenarios
where a function allocates the memory and the main just prints it.

Thanks for your replies.


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

#define CHAR_BIT 8

This is defined in the standard header limits.h. You should include
that rather than assuming its value.
char *print8bits(int8_t a){
int8_t mask = 1;
char retval[] = "00000000\0";
int i, j = sizeof(a)* CHAR_BIT -1;

Since int8_t cannot have padding, I don't believe sizeof a can ever be
greater than 1.

If CHAR_BIT is greater than 8, you will attempt to store outside the
bounds of retval. Perhaps retval should be defined as
char retval[CHAR_BIT+1] = '\0';
and set with
memset(retval, '0', CHAR_BIT);

for(i = 0; i < sizeof(int8_t) * CHAR_BIT; i++)

You don't use i in the loop. Wouldn't it be easier to make j the
control variable?
{
if (a & mask)
retval[j] = '1';
mask = mask << 1;

mask starts out as 0x01 and is shifted to 0x02, 0x04, 0x08, 0x10,
0x20, and 0x40 in turn. During the seventh iteration of the loop, it
will be shifted to 0x80 which, when stored back into mask, may result
in an implementation defined value (with a different bit pattern) or
an implementation defined signal. During the eighth iteration it will
be shifted to 0x100 which, even though it is never used, may also
raise a signal. mask should be either uint8_t or a larger integer
type.
j--;
}
printf("Printing 8 bit integer = %s, %d\n", retval, a); //prints
fine

What you have will work because a will be promoted to int but:

int8_t is defined in stdint.h You included inttypes.h instead (which
is ok since inttypes.h includes stdint.h) yet you never take advantage
of the addition definitions inttypes.h provides, in particular PRID8,
the format for printing an int8_t.
printf("Address of retval = %p\n", retval); //Address are same in
both the places

retval should be cast to void*. While char* and void* have the same
representation, there is no requirement that they be passed using the
same technique.
return retval;
}

int main()
{
char *temp = print8bits(-15); // I want to print -15 as 11110001.
printf("Address of retval in main = %p\n", temp); //Address are
same in both the places

This actually invoked undefined behavior because the value in temp
became magically indeterminate as soon as retval ceased to exist. It
is only bad luck that the value appeared to be unchanged.
printf("Printing 8 bit integer: %s\n", temp); // prints garbage

As others have explained.
 
G

Guest

A quick question, how can I initialize value using malloc(). For
example, following is not valid.

char *retval = malloc(CHAR_BIT + 1);
retval = "00000000\0";

Is there a terse way of doing it ? I know I could do in a for loop by
assigning retval a value of '0' each time. I wanted to learn a
smarter way.


memset()?
 
E

Eric Sosman

William said:
You should not define this. Allow the implementation to
define it for you by #include-ing limits.h.

A closer look at his code shows that the assumption
CHAR_BIT==8 is built-in, and in fact the code will not
compile if CHAR_BIT!=8 (because in that case int8_t will
not exist). Under the circumstances, the value of his
expression

sizeof(int8_t) * CHAR_BIT

.... is pretty much a foregone conclusion, carrying the
appearance of portability without the actuality.
 
R

Richard

A quick question, how can I initialize value using malloc(). For
example, following is not valid.

char *retval = malloc(CHAR_BIT + 1);
retval = "00000000\0";

Is there a terse way of doing it ? I know I could do in a for loop by
assigning retval a value of '0' each time. I wanted to learn a
smarter way.


memset()?


Is a char a byte? I forget everytime I pop into c.l.c.
 
F

Flash Gordon

Eric said:
A closer look at his code shows that the assumption
CHAR_BIT==8 is built-in, and in fact the code will not
compile if CHAR_BIT!=8 (because in that case int8_t will
not exist). Under the circumstances, the value of his
expression

sizeof(int8_t) * CHAR_BIT

... is pretty much a foregone conclusion, carrying the
appearance of portability without the actuality.

I would still say that doing a #define of CHAR_BIT was a bad idea, the
code might one day be changed such that another value from limits.h is
needed. Including limits.h and doing a compile time test of CHAR_BIT
would not be unreasonable.

Personally I don't think it is unreasonable a lot of the time to write
code that depends on 8 bit bytes since there is no real chance of a lot
of code being ported to embedded systems (does anyone want an enterprise
level accounts package running on a toaster?) and for some applications
it would require significant extra work.
 
K

Keith Thompson

Eric Sosman said:
Maybe I wasn't clear. The O.P.'s code *already* depends on
eight-bit bytes, and that dependency is explicit in the use of
the int8_t type. Since an int8_t incorporates at least one byte
(6.2.6.1p2) and has no padding bits (7.18.1.1), CHAR_BIT must be
a divisor of eight. And since CHAR_BIT>=8 (5.2.4.2.1), we must
have CHAR_BIT==8 and sizeof(int8_t)==1 if int8_t exists at all.

In short, the O.P.'s `sizeof(int8_t) * CHAR_BIT' is either
a compile error or `1 * 8', always. The number of bits in an
int8_t turns out to be equal to CHAR_BIT, but doesn't "depend"
on the value of CHAR_BIT; the shoe is on the other foot, as it
were, and the existence of int8_t constrains CHAR_BIT. That's
what I meant by "appearance of portability without the actuality:"
by using CHAR_BIT he appears to be allowing for chars of other
widths, but by using int8_t he doesn't actually do so. Dragging
CHAR_BIT into the picture does not improve portability at all,
so I see no point in using the macro here. That's all.

And #define'ing CHAR_BIT in your own code (as the OP did), rather than
obtaining the definition via "#include <limits.h>", almost *never*
makes any sense. Adding "#define CHAR_BIT 16" to your own code won't
magically give you 16-bit bytes.
 
F

Flash Gordon

Eric said:
Maybe I wasn't clear. The O.P.'s code *already* depends on
eight-bit bytes, and that dependency is explicit in the use of
the int8_t type. Since an int8_t incorporates at least one byte

widths, but by using int8_t he doesn't actually do so. Dragging
CHAR_BIT into the picture does not improve portability at all,
so I see no point in using the macro here. That's all.

"You can have it any color you like, as long as it's black."

OK, I can see that. However, I would still say that a #define of it is
making matters worse. It causes problems if you later change the code
such that you need another value from limits.h as well as suggesting a
lack of understanding of C.
 
G

Guest

A quick question, how can I initialize value using malloc(). For
example, following is not valid.
char *retval = malloc(CHAR_BIT + 1);
retval = "00000000\0";
Is there a terse way of doing it ? I know I could do in a for loop by
assigning retval a value of '0' each time. I wanted to learn a
smarter way.

memset()?

Is a char a byte? I forget everytime I pop into c.l.c.


maybe
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,431
Messages
2,571,678
Members
48,796
Latest member
Greg L.

Latest Threads

Top