2

In reference to question and the answer here: Can I use this method so that the solution will be platform independent.

char *buff = (char*) malloc(sizeof(unsigned long)*8);
sprintf(buff, "%lu", unsigned_long_variable);

Here I am getting the value of buffer length as it will similar to unsigned long variable. Is this approach correct?

Community
  • 1
  • 1
adnaan.zohran
  • 809
  • 9
  • 21
  • 1
    Please [see why not to cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. – Sourav Ghosh Jul 21 '15 at 10:12
  • @SouravGhosh : thanks for the tip but is it relevant to my doubt?! I seek to understand whether the above approach is platform independent or not. And I want to understand if above method is correct or not. If incorrect, purely for understanding and learning!! – adnaan.zohran Jul 21 '15 at 10:16
  • 3
    @SouravGhosh, is that a habit of yours posting this irrelevant and highly debatable advice everywhere? – gnasher729 Jul 21 '15 at 10:45
  • @SouravGhosh: The thing is that there are arguments *for* casting the return value as well (see the comments under unwind's accepted answer), and posting that link under every single question that does cast `malloc()` is... well, what would you say if I'd post a link to my favourite coding style guide under every question that violates it? – DevSolar Jul 21 '15 at 13:53
  • @DevSolar Sir, maybe to the "regular"s like us, it's repetitive, but I consider this to be useful for people landing on particular question, say via Google search. Also, I never said that is a **must**. I've added link to the question itself (to avoid any possible bias) where the most upvoted answer favours the comment body. :-) – Sourav Ghosh Jul 21 '15 at 14:10

5 Answers5

3

Don't even try to calculate the buffer size.

Start with snprintf, which will tell you safely how many characters are needed. Then you know how many bytes to allocate to print safely.

Since this is a few lines of code that you don't want to repeat again and again, write a function malloc_printf that does exactly what you want: In that function, call snprintf with a NULL destination, then malloc the buffer, sprintf into the malloc buffer, and return it. To make it faster and to often avoid two snprintf and sprintf calls, write into a buffer of 256 chars first which is often enough.

So your final code would be

char* buff = malloc_printf ("%lu", unsigned_long_variable);

Also does quick, safe and easy string concatenation using the format %s%s, for example.

undur_gongor
  • 15,657
  • 5
  • 63
  • 75
gnasher729
  • 51,477
  • 5
  • 75
  • 98
1

The C standard doesn't put an upper limit to the number of bits per char.

If someone constructs a C compiler that uses for example 2000 bits per char the output can overflow the buffer.

Instead of 8 you should use CHAR_BIT from limits.h.

Also, note that you need (slighly less than) 1 char per 3 bits and you need 1 byte for the string terminator.

So, something like this:

#include <limit.h>

char *buff = malloc(1 + (sizeof(unsigned long) * CHAR_BIT + 2) / 3);
sprintf(buff, "%lu", unsigned_long_variable);
Community
  • 1
  • 1
Klas Lindbäck
  • 33,105
  • 5
  • 57
  • 82
1

No, this is not the right way to calculate the buffer size.

E.g. for 4 byte unsigned longs you have values up to 2^32-1 which means 10 decimal digits. So your buffer needs 11 chars.

You are allocating 4 * 8 = 32.

The correct formula is

ceil(log10(2^(sizeof(unsigned long) * CHAR_BIT) - 1)) + 1

(log10 denotes the decimal logarithm here)

A good (safe) estimation is: 

(sizeof(unsigned long) * CHAR_BIT + 2) / 3 + 1

because log10(2) is less than 0.33.

undur_gongor
  • 15,657
  • 5
  • 63
  • 75
1

You want to know how many characters are needed to represent the largest possible unsigned long. Correct?

To that end, you are trying to calculate the largest possible unsigned long:

sizeof(unsigned long)*8

That is faulty in several ways. For one, sizeof returns multiples of char, which need not be 8 bit. You should multiply with CHAR_BIT (from <limits.h>) instead. But even that is not necessary, because that very same header already does provide the largest possible value -- UCHAR_MAX.

Then you're making a mistake: Your calculation gives the size of the integer representation of unsigned long in bits. What you want is the size of the string representation in characters. This can be achieved with the log10() function (from <math.h>):

log10( UCHAR_MAX )

This will give you a double value that indicates the number of (decimal) digits in UCHAR_MAX. It will be a fraction, which you need to round up (1) (ceil() does this for you).

Thus:

#include <math.h>
#include <stdlib.h>
#include <limits.h>

int main()
{
    char * buff = malloc( ceil( log10( UCHAR_MAX ) ) + 1 );
    //...
}

All in all, this is quite dodgy (I made two mistakes while writing this out, shame on me -- if you make mistakes when using this, shame on you). And it requires the use of the math library for something that snprintf( NULL, ... ) can do for you more easily, as indicated by the Q&A you linked to.

(1): log10( 9999 ) gives 3.9999565... for the four-digit number.

DevSolar
  • 67,862
  • 21
  • 134
  • 209
  • 1
    But contrary to the explanations, your code does **not** round-up. Or, you forgot the trailing 0. Besides, C's `log` is the natural logarithm. We need `log10' . – undur_gongor Jul 21 '15 at 11:03
  • 1
    Generic advice: Whenever a number appears in your source code that is not `1` or `0`, that's a code smell, a.k.a. [unnamed numerical constant](https://en.wikipedia.org/wiki/Magic_number_%28programming%29#Unnamed_numerical_constants). It is an indicator that you're doing something wrong. – DevSolar Jul 21 '15 at 11:07
  • @undur_gongor: Hurrr... another lesson of "test before post". I intended the `+ 1` to do the round-up, and promptly forgot the null byte terminator. :-D – DevSolar Jul 21 '15 at 11:08
  • I saw a lot of code containing things like `#define TWO 2` caused by mis-applying that advice. – undur_gongor Jul 21 '15 at 11:17
  • @undur_gongor: I even saw `#define TWO 3` once. No kidding. But you can mis-apply *any* advice if you put your mind to it. And I said "code smell", which means you *might* be doing something wrong, not that you *are* doing something wrong. (You'd need `2` to check for even numbers, for example.) – DevSolar Jul 21 '15 at 11:19
1

Short answer: 

#define INTEGER_STRING_SIZE(t) (sizeof (t) * CHAR_BIT / 3 + 3)

unsigned long x;
char buf[INTEGER_STRING_SIZE(x)];
int len = snprintf(buf, sizeof buf, "%lu", x);
if (len < 0 || len >= sizeof buf) Handle_UnexpectedOutput();

OP's use of sizeof(unsigned long)*8 is weak. On systems where CHAR_BIT (the # of bits per char) is large (it must be at least 8), sizeof(unsigned long) could be 1. 1*8 char is certainly too small for 4294967295 (the minimum value for ULONG_MAX).

Concerning: sprintf()/snprintf() Given locale issues, in theory, code may print additional characters like 4,294,967,295 and so exceed the anticipated buffer. Unless very tight memory constraints occur, recommend a 2x anticipated sized buffer.

char buf[ULONG_STRING_SIZE * 2];  // 2x
int len = snprintf(buf, sizeof buf, "%lu", x);

The expected maximum string width of printing some unsigned integer is ceil(log10(unsigned_MAX)) + 1. In the case of of unsigned long, the value of ULONG_MAX certainly does not exceed pow(2,sizeof (unsigned long) * CHAR_BIT) - 1 so code could use:

#define LOG10_2 0.30102999566398119521373889472449
#define ULONG_STRING_SIZE (sizeof (unsigned long) * CHAR_BIT * LOG10_2 + 2)
// For greater portability, should use integer math.
#define ULONG_STRING_SIZE (sizeof (unsigned long) * CHAR_BIT / 3 + 2)
// or more precisely
#define ULONG_STRING_SIZE (sizeof (unsigned long) * CHAR_BIT * 28/93 + 2)

The short answer used +3 in case a signed` integer was specified.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256