Comment On One Step Forward...

When working with embedded systems, it is sometimes important to do dirty little hacks. There are many ways to call upon the dark magic, though a large number of them are pointer or string tricks, often sacrificing readability and portability for memory or performance. There is a local benefit. [expand full text]
« PrevPage 1 | Page 2 | Page 3 | Page 4Next »

Re: One Step Forward...

2007-02-14 09:02 • by Dominic (unregistered)
The return value of sprintf is the number of characters written...

Re: One Step Forward...

2007-02-14 09:04 • by Aaron (unregistered)
bonus points? that depends.... is it because we're assuming that "N" is indeed an integer value? :D

(Given some of the horrid programming we see on this site, it wouldn't at all surprise me to see someone trying to use a char value as an array size.)

FRIST!

Re: One Step Forward...

2007-02-14 09:04 • by ... (unregistered)
I believe that should read "sprintf"

Re: One Step Forward...

2007-02-14 09:05 • by Michael (unregistered)
Why putting in the \n\r in the first place?
tsktsktsk, and what about buffer overflow?

captcha: stinky - like a kinky malinki

Re: One Step Forward...

2007-02-14 09:08 • by Gabelstaplerfahrer
I think it shows a complete and utter lack of understanding of the sprintf and its syntax...
And for the bonus points: of course the length can be read from N. But it isn't needed in the first place.

Re: One Step Forward...

2007-02-14 09:09 • by 8an (unregistered)
You'll get bonus points if you can figure out why I think strlen is redundant.

I know! sprintf returns the number or characters printed, so it could be simplified to:
buffer[sprinf(buffer, "%s: %s\n\r",header,value) - 2] = 0;

(Just kidding...)

Re: One Step Forward...

2007-02-14 09:13 • by john doe (unregistered)
120383 in reply to 120381
Gabelstaplerfahrer:
I think it shows a complete and utter lack of understanding of the sprintf and its syntax...
And for the bonus points: of course the length can be read from N. But it isn't needed in the first place.

Of course not. That's the length of the entire buffer, not of the string which is written into it (or perhaps past it, since snprintf isn't used).

Re: One Step Forward...

2007-02-14 09:17 • by BA (unregistered)
I think the issue is that he is manually attaching the carriage return, then stripping it out right after.

Hence, One Step Forward... (One Step Back).

Re: One Step Forward...

2007-02-14 09:19 • by ComputerForumUser (unregistered)
The comment says remove the carriage return but it really removes the newline.

Also that thing with them being put in by the previous line.

Re: One Step Forward...

2007-02-14 09:20 • by Hello Kitty (unregistered)
And of course the comment says "remove carrigage return", while the charater at n-2 is the LINE FEED (\n). Carriage return is at n-1.

Re: One Step Forward...

2007-02-14 09:21 • by akatherder
buffer[len-2] = 0;

What is that attempting to do??? Replace one element of the array with the number 0? Doesn't it just leave the rest of the array alone (which would be the rest of the carriage return line feed)?

And shouldn't this be done with a 2D array? Maybe it has something to do with saving space in an embedded system, but if someone names a key or value with a ":" you appear to be screwed.

I think I got herpes just from reading this. IT BURNS!

Re: One Step Forward...

2007-02-14 09:22 • by Rowboat (unregistered)
The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it.

Re: One Step Forward...

2007-02-14 09:24 • by A Guy (unregistered)
Really this should be:

snprintf(buffer, N-1, "%s: %s",header,value);


bounds checking is good

Re: One Step Forward...

2007-02-14 09:25 • by Pitabred (unregistered)
120392 in reply to 120388
I think the 0 was trying to move the null-termination forward. But it's still not null, it's just the integer value 0. Null should be \0.

And the strlen is extraneous because the initial string is a null-terminated string, so it'll always return N as the size.

I think. It's been a while since I've done C.

Re: One Step Forward...

2007-02-14 09:27 • by Felix (unregistered)
Plus, it's \r\n, not \n\r.

(On the other side, i've lately seen embedded code which had a /n/r in place. Sometimes i wonder how these devices can work at all.)

Re: One Step Forward...

2007-02-14 09:30 • by SeekerDarksteel
120396 in reply to 120392
This word, null....i do not think it means what you think it means.

Re: One Step Forward...

2007-02-14 09:37 • by Frank Mitchell (unregistered)
According to http://www.cppreference.com/stdio/sprintf.html sprintf returns the number of characters read. (How could it know the total size of the buffer?)

Oh, and snprintf isn't part of the standard C library, as far as I can tell. Many embedded devices are restricted to ANSI C (or less).

Re: One Step Forward...

2007-02-14 09:37 • by Marco Schramp (unregistered)
120401 in reply to 120392
Pitabred:
I think the 0 was trying to move the null-termination forward. But it's still not null, it's just the integer value 0. Null should be \0.

And the strlen is extraneous because the initial string is a null-terminated string, so it'll always return N as the size.

I think. It's been a while since I've done C.


No assigning zero is correct. Actually '\0' == (char) 0 is true.

Re: One Step Forward...

2007-02-14 09:39 • by Lumpy (unregistered)
I know! It's not i18n compliant; the code should be using wchar_t and wsprintf.

Re: One Step Forward...

2007-02-14 09:41 • by Ev (unregistered)
Hmm, there are a few wtf, but all minor imho:
1. \n\r, which should probably be \r\n
2. First placing the \n\r in the buffer with an sprintf call, then removing it again...
3. Yeah, the strlen could of course be removed by just getting the returned integer from sprin[t]f

A few responses of people (some mentioned before):
1. No you can NOT use N as the strlen. N is the size of the buffer where the data will be stored. It's like getting the capacity of the glass to find out how much water you have, while maybe you only put a few drops in it.
2. The buffer overflow might not be a problem, it might very well be checked before. Although I doubt it...
3. 0 will be put in the buffer. This is ASCII 0 which should (nearly) always be a null-terminator of the string. Of course, '\0' should be used, but I have to admit I've used "= 0" as well. So yes, both \n\r will be "removed" (effectively).
4. \r does not return to the begging of the string. Yes, usually it jumps to the beginning of the line before printing the rest, when printing. Eg: printf("abcd\r1234\n"); often displays "1234". But still, the data is very much in the buffer.

So a bit of a wtf, but not a great one IMHO

Re: One Step Forward...

2007-02-14 09:43 • by Ev (unregistered)
120404 in reply to 120401
Marco Schramp:
Pitabred:
I think the 0 was trying to move the null-termination forward. But it's still not null, it's just the integer value 0. Null should be \0.

And the strlen is extraneous because the initial string is a null-terminated string, so it'll always return N as the size.

I think. It's been a while since I've done C.


No assigning zero is correct. Actually '\0' == (char) 0 is true.


Hmmm I just posted a reply in which I stated it was nearly always true.
But you're right. In fact, if you look at code you may assume to be valid, you often run accross something like this:
while(buf[x++] = buf2[y++]);
or:
if(*buf)

Which would, of course, only work if the null-terminator is actually 0.

Re: One Step Forward...

2007-02-14 09:45 • by viraptor
Ok - i'm scared... comments are "The Real WTF":

120377: "is it because we're assuming that "N" is indeed an integer value? :D"
120381: "of course the length can be read from N. But it isn't needed in the first place."
120388: "Replace one element of the array with the number 0? Doesn't it just leave the rest of the array alone (which would be the rest of the carriage return line feed)? And shouldn't this be done with a 2D array?"
120389: "The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it."
120392: "But it's still not null, it's just the integer value 0. Null should be \0. And the strlen is extraneous because the initial string is a null-terminated string, so it'll always return N as the size"

I'm sorry - you all fail :)
strlen counts chars up to '\0' (==0) and doesn't care about "\r\n" and will return value, which is already known from sprintf()
What's wrong with you guys today? (V-day hit hard, or sth?)
+ yeah... snprintf should be used
+ depending on size of memory in that chip, maybe it's good to skip snprintf and write this simple string concat yourself. (yes - linking *printf family can take a lot of your precious free memory if you do something on 1KB flash)

Addendum (2007-02-14 09:52):
+ yeah - forgot about main issue - why write \r\n at all if you remove it afterwards

Re: One Step Forward...

2007-02-14 09:45 • by Disgrundled postal worker (unregistered)
120406 in reply to 120389
This website is entertaining, funny to read.

The comments often leave me unsure whether to laugh or scream loudly and move to a little cabin in the woods, staying well away from the IT business for ever.

There's always an early poster who gets it, but his post is ignored, and followed by a long thread, where at least 9 out of 10 posts are of the form:

<insert insult>
The (actual issue|real WTF) is:
<insert utter nonsense>

I don't think I'll be reading the comments on this site again

Re: One Step Forward...

2007-02-14 09:46 • by Mr. 45 cm (unregistered)
sprinf(buffer, "%s: %s\n\r",header,value);

The real wtf is that he should use sprintf

Re: One Step Forward...

2007-02-14 09:50 • by Leo (unregistered)
People,, the WTF is that at the line

sprinf(buffer, "%s: %s\n\r",header,value);

He WRITES a CR, then removes it. If he didn't want the CR, the line should be

sprinf(buffer, "%s: %s\n",header,value);

Mind that he removes, the character before the last, wich is the \r, NOT the \n.

No, bound checking is unnecessary if you know, FOR SURE, that you won't overstep the string.

akatherder, yes, he's doing exacly that. You see, the 0, or \0, is the character that marks the end of the string. A LOT of C functions will operate the string untill they find a \0. strlen, for example, will count the number of characters until it finds a \0, thus returning the number of characters in the string. Mind that the \0 IS part of the string and should "fit" inside the buffer.

Re: One Step Forward...

2007-02-14 09:51 • by iwpg
120409 in reply to 120405
viraptor:
Ok - i'm scared... comments are "The Real WTF":

120389: "The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it."

I'm 99.9% sure this one was a joke.

Re: One Step Forward...

2007-02-14 09:56 • by Bima (unregistered)
120411 in reply to 120405
viraptor:
Ok - i'm scared... comments are "The Real WTF":

120377: "is it because we're assuming that "N" is indeed an integer value? :D"
120381: "of course the length can be read from N. But it isn't needed in the first place."
120388: "Replace one element of the array with the number 0? Doesn't it just leave the rest of the array alone (which would be the rest of the carriage return line feed)? And shouldn't this be done with a 2D array?"
120389: "The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it."
120392: "But it's still not null, it's just the integer value 0. Null should be \0. And the strlen is extraneous because the initial string is a null-terminated string, so it'll always return N as the size"

I'm sorry - you all fail :)
There are often many more WTFs in the comments then there are in the original submissions. I've come to the conclusion that many posters are just trying to out-troll each other and are not really serious about their posts. This is perhaps an unduely charitable and optimistic view of the programming community, but the alternative view saddens me even more.

Re: One Step Forward...

2007-02-14 09:58 • by snoofle
120412 in reply to 120409
iwpg:
viraptor:
Ok - i'm scared... comments are "The Real WTF":

120389: "The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it."

I'm 99.9% sure this one was a joke.

Or perhaps just anonymized to reflect the flavor of the wtf that was submitted...

Re: One Step Forward...

2007-02-14 09:59 • by Mike5 (unregistered)
WTF is a "sprinf" ?

Re: One Step Forward...

2007-02-14 10:01 • by snoofle
120414 in reply to 120411
Bima:
viraptor:
Ok - i'm scared... comments are "The Real WTF":

120377: "is it because we're assuming that "N" is indeed an integer value? :D"
120381: "of course the length can be read from N. But it isn't needed in the first place."
120388: "Replace one element of the array with the number 0? Doesn't it just leave the rest of the array alone (which would be the rest of the carriage return line feed)? And shouldn't this be done with a 2D array?"
120389: "The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it."
120392: "But it's still not null, it's just the integer value 0. Null should be \0. And the strlen is extraneous because the initial string is a null-terminated string, so it'll always return N as the size"

I'm sorry - you all fail :)
There are often many more WTFs in the comments then there are in the original submissions. I've come to the conclusion that many posters are just trying to out-troll each other and are not really serious about their posts. This is perhaps an unduely charitable and optimistic view of the programming community, but the alternative view saddens me even more.

I agree. However, I think it may go a little deeper. I've been around since long before the internet came along enabling every Tom, Dick and Harry to spout off, and have noticed that once folks have access to a forum, they want to respond as if they were in the same room as person who wrote the post to which they are responding; they just shoot from the hip.

Seriously, could people actually be as illiterate (incompetent) as the spelling (comments) on this forum would suggest? (God, I hope not *shudders* )

Re: One Step Forward...

2007-02-14 10:02 • by Mike5 (unregistered)
120415 in reply to 120389
Rowboat:
The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it.


Are you on something?

Re: One Step Forward...

2007-02-14 10:02 • by joerbanno (unregistered)
120416 in reply to 120387
Hello Kitty:
And of course the comment says "remove carrigage return", while the charater at n-2 is the LINE FEED (\n). Carriage return is at n-1.




Hmmmn I always thought it was CR LF...
0d0a...

Re: One Step Forward...

2007-02-14 10:06 • by snoofle
120417 in reply to 120416
joerbanno:
Hello Kitty:
And of course the comment says "remove carrigage return", while the charater at n-2 is the LINE FEED (\n). Carriage return is at n-1.




Hmmmn I always thought it was CR LF...
0d0a...

To be fair, I suppose the implementation/values of CR and LF depend upon the character set/language. Think about languages that are written vertically or right to left.

WTF would we do to handle a language that someone designed to print diagonally on the page? Wheee!

Re: One Step Forward...

2007-02-14 10:09 • by Rowboat (unregistered)
120419 in reply to 120415
Mike5:
Rowboat:
The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it.


Are you on something?


Yes. I'm on having been too distracted after a long day at work to think properly and also on having tested this the wrong way before posting.

Re: One Step Forward...

2007-02-14 10:10 • by Rowboat (unregistered)
...I mean, it's a joke! A joke!

Re: One Step Forward...

2007-02-14 10:12 • by mav (unregistered)
120421 in reply to 120388
What on earth are you talking about? 2D array what, huh? And the reason for putting in the "number 0" is effectively null terminating the c string. A perfectly valid thing to do in many cases.

Sigh. Reading some of these comments is even more painful than the original code.


CAPTCHA: burned, just like my pee. :-(

Re: One Step Forward...

2007-02-14 10:16 • by Will (unregistered)
120422 in reply to 120417
snoofle:

WTF would we do to handle a language that someone designed to print diagonally on the page?


Bomb the country where the language originated into submission.

Re: One Step Forward...

2007-02-14 10:16 • by joerbanno (unregistered)
120423 in reply to 120417
snoofle:
joerbanno:
Hello Kitty:
And of course the comment says "remove carrigage return", while the charater at n-2 is the LINE FEED (\n). Carriage return is at n-1.




Hmmmn I always thought it was CR LF...
0d0a...

To be fair, I suppose the implementation/values of CR and LF depend upon the character set/language. Think about languages that are written vertically or right to left.

WTF would we do to handle a language that someone designed to print diagonally on the page? Wheee!


Hmm shouldn't the OS handle that? Oh wait it's embedded..

Re: One Step Forward...

2007-02-14 10:17 • by joerbanno (unregistered)
120424 in reply to 120417
snoofle:
joerbanno:
Hello Kitty:
And of course the comment says "remove carrigage return", while the charater at n-2 is the LINE FEED (\n). Carriage return is at n-1.




Hmmmn I always thought it was CR LF...
0d0a...

To be fair, I suppose the implementation/values of CR and LF depend upon the character set/language. Think about languages that are written vertically or right to left.

WTF would we do to handle a language that someone designed to print diagonally on the page? Wheee!


Hmm shouldn't the OS handle that? Oh wait it's embedded..

Re: One Step Forward...

2007-02-14 10:17 • by KoFFiE (unregistered)
120425 in reply to 120390
A Guy:
Really this should be:

snprintf(buffer, N-1, "%s: %s",header,value);


bounds checking is good


This should do, no need for the "-1":

snprintf(buffer, N, "%s: %s",header,value);



From the snprintf manpage:
snprintf and vsnprintf do not write more than size bytes (including the trailing '\0'), and return -1 if the output was truncated due to this limit.

Re: One Step Forward...

2007-02-14 10:18 • by Eduardo Habkost (unregistered)
120426 in reply to 120420
Rowboat:
...I mean, it's a joke! A joke!


Damn. My sarcasm detector is not working properly today. I can't tell if you were serious or not.

If you were serious, that was _really_ funny. But if that was supposed to be a joke, it was so absurd that it wasn't funny. 8)

Re: One Step Forward...

2007-02-14 10:19 • by ComputerForumUser (unregistered)
<insert insult>

The (actual issue|real WTF) is:
<insert utter nonsense>


This could be written as one line:

The real WTF is your mother >:p

Re: One Step Forward...

2007-02-14 10:20 • by akatherder
120428 in reply to 120408
Leo:

akatherder, yes, he's doing exacly that. You see, the 0, or \0, is the character that marks the end of the string. A LOT of C functions will operate the string untill they find a \0. strlen, for example, will count the number of characters until it finds a \0, thus returning the number of characters in the string. Mind that the \0 IS part of the string and should "fit" inside the buffer.


At first I thought the code was retarded and it would just replace the \r with a "0". That's pretty clever, although my lack of C knowledge probably makes it seem more impressive than it is. At any rate it's handy to know.

Re: One Step Forward...

2007-02-14 10:23 • by mariush (unregistered)
What's even funnier is having links like the one below on this site:

http://thedailywtf.com/Articles/One_Step_Forward_0x2e__0x2e__0x2e_.aspx

Re: One Step Forward...

2007-02-14 10:26 • by anonymous guy (unregistered)
120432 in reply to 120411
Bima:
viraptor:
Ok - i'm scared... comments are "The Real WTF":

120377: "is it because we're assuming that "N" is indeed an integer value? :D"
120381: "of course the length can be read from N. But it isn't needed in the first place."
120388: "Replace one element of the array with the number 0? Doesn't it just leave the rest of the array alone (which would be the rest of the carriage return line feed)? And shouldn't this be done with a 2D array?"
120389: "The actual issue is that the \r returns to the beginning of the string before writing the final 0 - which means that strlen will always return 0. So the code writes an effectively empty string, then corrupts the memory just before it."
120392: "But it's still not null, it's just the integer value 0. Null should be \0. And the strlen is extraneous because the initial string is a null-terminated string, so it'll always return N as the size"

I'm sorry - you all fail :)
There are often many more WTFs in the comments then there are in the original submissions. I've come to the conclusion that many posters are just trying to out-troll each other and are not really serious about their posts. This is perhaps an unduely charitable and optimistic view of the programming community, but the alternative view saddens me even more.

Hey! The comments are the best part of thedailywtf; sort of a meta-wtf. And no, I really don't they're trolls.

Although in this case, I think it is possible (especially in embedded systems) that sprintf does not obey the standard. Used to be all sorts of different return values from the printf functions from different libraries. In fact, as recently as MS's VC++ 6 for the *nprintf() functions, returned -1 if the length was exceeded, while GNU libc returned the length that would've been printed. I haven't used VC 7, I don't know about that. So really, the only wtf here is putting in \n\r (backwards) and then removing it. Not being skeptical of the portability of the return value by using strlen().

Re: One Step Forward...

2007-02-14 10:37 • by Mr. Man (unregistered)
When the char array is allocated:
char buffer[N];
it is not initialized, so you have N characters in the array but they are not necessarily set to '\0'.

So calling strlen(buffer) will count from buffer[0] until the first '\0' encountered, which may be well after buffer[N].

Not only is calling strlen() redundant, but it's downright dangerous here.

Re: One Step Forward...

2007-02-14 10:48 • by slamb (unregistered)
The real WTF is that someone gave these clowns a C-99 compiler, while my platform still doesn't have one.

Re: One Step Forward...

2007-02-14 10:54 • by Southern (unregistered)
I thought strlen only stopped 'counting' when it found a '\0' .. would have to see how that works ... I guess he may have initialized the entire array with '\0' but then I'd have to wonder if that funcion is reusable.

To sum up, I'd have used strtok instead if I were to remove the first ocurrence of '\n'. But maybe I would have done everything in a different way.

Re: One Step Forward...

2007-02-14 10:57 • by seconddevil
120442 in reply to 120435
Mr. Man:
When the char array is allocated:
char buffer[N];
it is not initialized, so you have N characters in the array but they are not necessarily set to '\0'.

So calling strlen(buffer) will count from buffer[0] until the first '\0' encountered, which may be well after buffer[N].

Not only is calling strlen() redundant, but it's downright dangerous here.

From
man sprintf
:


Return value
Upon successful return, these functions return the number of characters printed (not including the trailing ’\0’ used to end output to strings). The functions
snprintf() and vsnprintf() do not write more than size bytes (including the trailing ’\0’). If the output was truncated due to this limit then the return value
is the number of characters (not including the trailing ’\0’) which would have been written to the final string if enough space had been available. Thus, a return
value of size or more means that the output was truncated. (See also below under NOTES.) If an output error is encountered, a negative value is returned

Re: One Step Forward...

2007-02-14 11:07 • by Marcin (unregistered)
The real WTF is the use of printf, which must be interpreted at runtime, rather than using a manually-coded "format." Even better would have been if the buffers had been made physically proximate, so that there would be no need to concatenate them.
« PrevPage 1 | Page 2 | Page 3 | Page 4Next »

Add Comment