CODEWITHSUNDEEP
cstringprintf
Skegg
Skegg

Reputation: 890

snprintf to concatenate multiple strings don't work as expected

It has been pointed out in quite a few places that to concatenate multiple strings it is more easy and also elegant to use snprintf instead of running multiple runs of strcat. But I've been getting incorrect results because of that. In this little snippet below which is part of program to convert 12hr to 24hr time, I first separate the hr,min and sec part and after converting the time, re-join the strings again.

here's the snippet

/*  
char *timestr = strcat(hr, min);
timestr = strcat(timestr, sec);
timestr = strcat(timestr, AMPM);
*/

///*    
char *timestr = (char *)malloc(20);
snprintf(timestr, sizeof(timestr), "%s:%s:%s", hr,min,sec);
//*/

the first version works fine but the bottom one doesn't. Here's the expected output which is what I see with strcat

(gdb) p timestr
$1 = 0x7fffffffe8cd "190545PM"

this is what I see with snprintf

(gdb) p timestr
$1 = 0x801007040 "19:05:4"

I am unable to fathom what is going wrong. Isn't printing to the stream supposed to take care of the trailing NULL character?

Upvotes: 0

Views: 1082

Answers (3)

AndreyS Scherbakov
AndreyS Scherbakov

Reputation: 2788

You may use another sibling function, asprintf(...), to allocate a proper piece of memory automatically:

char* timestr;
asprintf(&timestr, "%s:%s:%s", hr,min,sec);

The original solution doesn't work because sizeof(char*) == 8 (bytes) rather than 20, it's the pointer size itself and it doesn't depend on contents pointed by that or this pointer.

Upvotes: 0

user2736738
user2736738

Reputation: 30926

In case of snprintf if the second argument is n then manual says

If the resulting string would be longer than n-1 characters, the remaining characters are discarded and not stored, but counted for the value returned by the function.

So what is the case here for n? sizeof(temp) is sizeof(char*) which is not the same as 20 bytes you allocated. You can't apply sizeof to a pointer to get the memory size it points to. (On a typical system sizeof(char*) varies).

Solution:

snprintf(timestr, 20, "%s:%s:%s", hr,min,sec);

or if you keep it in some variable (which is recommended).

const int size = 20;
int ret = snprintf(timestr, size, "%s:%s:%s", hr,min,sec);
if( ret < 0){
    fprintf(stderr,"Error in snprintf operation\n");
    exit(1);
}
if( ret >= size){
   fprintf(stderr,"Error: more characters than expected\n");
   exit(1);
}

Check the return value of snprintf to understand whether same error occured in that case negative number is returned.

Upvotes: 2

Pablo
Pablo

Reputation: 13580

sizeof(timestr) doesn't return you the number of bytes that you've allocated, it returns you the number of bytes that a pointer to char needs to be stored in memory. The correct call would be:

char *timestr = malloc(20);
snprintf(timestr, 20, "%s:%s:%s", hr,min,sec);

Note that snprintf will try to write at nost 20 characters (including the 0-terminating byte). If the resulting length is not large enough, timestr will be cropped. So it's better to get the number of bytes that you would need by passing NULL and size 0 to snprintf. In this case it will return how many character would be required. So

int len = snprintf(NULL, 0, "%s:%s:%s", hr, min, sec);
if(len < 0)
{
    fprintf(stderr, "could not execute snprintf\n");
    return; // or whatever, do not continue
}
char *timestr = malloc(len + 1);
if(timestr == NULL)
{
    // error handling
    return; // or whatever, do not continue
}

sprintf(timestr, "%s:%S:%s", hr, min, sec);

would give you the exact amount of bytes for the string.

Upvotes: 1

Related Questions