C string literal handling

Question

What's wrong with the following piece of code that the program crashes - give segmentation fault. I am using gcc.

uint8_t result = 1      

InsertRow("Name","Details of work",result);     

void InsertRow(char *Name, char *Description,uint8_t Result)   
{  
   char Buffer[500];  

   if(Result==1)   
      sprintf(Buffer,"<tr><td>%s </td> <td> %s </td> <td>  %s </td></tr>",Name,Description,Result);   
}

Consider using `snprintf` to prevent buffer overflows. – aschepler May 25 '12 at 11:31 — aschepler, May 25 '12 at 11:31

unwind · Accepted Answer · 2012-05-25T14:11:41.573

7

You're using the %s formatting specifier for an argument of type uint8_t, this should be %u, and you should cast the value to unsigned int to match. This saves you from having to care about the exact type and adjust the formatter (as commenters suggest).

Also, it's hard for us to know that the buffer is large enough, of course. If you have it, you can use snprinf() to avoid this.

edited May 25 '12 at 14:11

answered May 25 '12 at 11:28

unwind

391,730
64
469
606

1

The format should be `PRIu8` for `uint8_t`, `%hhu` is likely to be the right one, too. `%u` is for `unsigned int`. – Daniel Fischer May 25 '12 at 11:38
@Daniel: isn't `uint8_t` guaranteed to be promoted to exactly `unsigned int` as a vararg, though? So `%u` will print it. I agree it's best to use the format macro rather than try to puzzle that out. – Steve Jessop May 25 '12 at 12:48
1

@SteveJessop As I understand the _integer promotions_, "If an int can represent all values of the original type (as restricted by the width, for a bit-ﬁeld), the value is converted to an int;", it's promoted to `int`. So technically, it would be UB, but I don't think there's any implementation where it wouldn't work as intended. – Daniel Fischer May 25 '12 at 13:07
@Daniel: fair enough. There's a special case in `va_arg`, that it's OK when "one type is a signed integer type, the other type is the corresponding unsigned integer type, and the value is representable in both types". So I do think that `%u` is OK, but that fact that I already made one mistake trying to justify it shows that I shouldn't advocate it :-) IIRC the `printf`-style functions bizarrely state stronger restrictions on arguments than are required for varargs, so you're probably right it's UB even though it's difficult to imagine how an implementation could do anything other than work. – Steve Jessop May 25 '12 at 13:56
@SteveJessop Ah, that may make it defined behaviour. I'm not sure, though, language-legalese is not my first language. Work, it will almost certainly, whether defined or undefined behaviour. – Daniel Fischer May 25 '12 at 14:11
The one place I've seen integer promotion cause an issue in printf is when you promote a signed integer into a format you think of as unsigned (i.e. hex or octal) and it does sign extending. For example: `int8_t x=0xFF; uint8_t y=0xFF; printf("%x %x\n", x, y);` With regard to type safety (i.e. not causing segfaults), gcc is pretty good about warning you when you feed any *printf function bad arguments when you compile with `-Wall`. – Brian McFarland May 25 '12 at 15:44

score 1 · Answer 2 · answered May 25 '12 at 11:29

Here

sprintf(Buffer,"<tr><td>%s </td> <td> %s </td> <td>  %s </td></tr>",Name,Description,Result);

you are passing Result (which is of type uint8_t) as a pointer to char array

This means that the integral value will be inrepreted as a pointer which most likely will point to memory not accessible for you -- hence the seg. fault. You need to replace the third %s with the appropriate formatting flag to print the value as integer

Note: do not use %d directly in this case as your uint8_t type is not guaranteed to be the same size as int (most likely is not). You could use %d if you cast the value of Result to an int first ((int)Result)

score 0 · Answer 3 · answered May 25 '12 at 11:30

0

Result is not a char*, you probably want %d for that

answered May 25 '12 at 11:30

paquetp

1,639
11
19

C string literal handling

3 Answers3