0
char buf[256];
sprintf(buf, "It was %s\r\n", weather);
write(p->fd, buf, sizeof(buf));

The code above is a snippet of a large project.

buf is used to hold a number of different strings of different length. How do I know what to put in the write function? sizeof() just gives 256 I believe, because write just spits out a bunch of extra garbage characters.

Anto Jurković
  • 11,188
  • 2
  • 29
  • 42
User
  • 23,729
  • 38
  • 124
  • 207

3 Answers3

4

The version of the code using len should be:

char buf[256];
int len = snprintf(buf, sizeof buf, "It was %s\r\n", weather);

if ( len < 0 || len >= sizeof buf )
     // error handling, abort...

write(p->fd, buf, len);

Using sprintf is risky as it may cause a buffer overflow if weather is not fully under your control.

As mentioned in its documentation, the sprintf family can return a negative value if there is an error; and the returned value can be larger than the buffer size if the write would have not fitted in the buffer.

Another option covered by other answers is to omit checking len, and instead use strlen to find the length to send. Some would consider that unnecessarily inefficient. Further, in that case you should really check len anyway in case encoding fails which would result in strlen operating on garbage.

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
0

Solution is a combination of memset and strlen or sprintf return

char buf[256];
memset(buf,'\0',sizeof(buf));
sprintf(buf, "It was %s\r\n", weather);
write(p->fd, buf, strlen(buf));

OR

char buf[256];
    memset(buf,'\0',sizeof(buf));
    int len = sprintf(buf, "It was %s\r\n", weather);
    write(p->fd, buf, len);
Yasir Majeed
  • 739
  • 3
  • 12
  • I saw that on tutorialspoint.com but I didn't understand why I needed `memset`. – User Mar 30 '15 at 05:15
  • Because when you declare an array, all the indexes of arrays contain garbage values. And you don't want garbage values to effect you functionality. For example for strings, they must end on '\0' character in order to strlen() to work correctly. But if you don't do memset, the end point will contain some garbage character and strlen() will give you wrong result – Yasir Majeed Mar 30 '15 at 05:18
  • The `memset` is useful in the `strlen` version in case `sprintf` [fails](http://stackoverflow.com/questions/2948361/when-and-why-can-sprintf-fail/). This is so that `strlen` finds `0` instead of trying to read an uninitialized buffer. However you only need to set `buf[0] = 0;`, not the whole thing. – M.M Mar 30 '15 at 05:47
  • @Yasir Majeed "if you don't do memset, the end point will contain some garbage " is not correct. `sprintf()` will append the needed `'\0'`. Check the return value of `sprintf()` to detect failure. – chux - Reinstate Monica Mar 31 '15 at 21:53
0

You are correct that sizeof() does not do what you want.

How you determine how much valid data is actually in your buffer depends on how you put the data there. In your particular case, you could either use the return value of sprintf(), or you could use strlen() on the buffer. I would recommend the former, since sprintf() is going to return that value whether you use it or not. Either of those alternatives will exclude the string's trailing null byte, so be sure to add one to the length if you want to write that null, too.

If elsewhere you are filling the buffer by other means, then the appropriate mechanism for determining how many bytes to write may differ.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157