- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
frist?
Unfortunately, you have to do this sort of thing when you have an embedded system with no file system
Admin
You can still copy-paste battle-proven implementations from e.g. ucLibc, can't you? (Honest question, I'm not that familiar with embedded development.)
Admin
The code will also return 0 for "+2"
Admin
To be fair, 10 iterations of Newton Raphson should be plenty for sqrt(10000), except with a pathologically bad initial guess. It'll probably break down around sqrt(1000000), but you can fix it up by using an initial guess based on sneaking a look at the IEEE exponent.
Quadratic convergence FTW.
Admin
Yes, you can, license permitting.
Admin
I don't understand why atoi("") would trigger a memory violation. Is this a non-standard C in which strings aren't NUL-terminated? If not, it should check if 0 == '-' (which it isn't), and then go into the else branch, and since 0 < '0', it'd return zero. Am I missing something here?
Admin
If there are typos and grammos in the comments, start being afraid. It indicates a lack of attention to detail. While it is well understood that most people working in computing brag proudly of how dyslexic they are and how only boring old pedants gvie a danm about such stpuid things as speling and making sure the code has it's grammer correct, I'm afraid I have found by experience that dyslexic programmers and/or those careless about such details tend to produce the worst code.
Admin
Or you can just sqrt bit-by-bit; it's only slightly more complicated than long division.
Admin
Yeah. I don't see the [memory violation] either. It should return 0 in the else if... clause. The example is passing a null string, not a null pointer.
Admin
The function works pretty good if one removes the "else" of "else if(s[0] < '0' || s[0] > '9')"
Furthermore it still lacks the handling of an empty string.
Admin
Remy, as a second exhibit, the insane sqrt function would be great.
Easy reader : I don't think the code qualifies as square. There are so many edge cases it looks like a dull and rusty saw blade.
Admin
Sure, that's a bad atoi, but there isn't really a good atoi. The standard atoi returns 0 when it can't parse an integer, so there's no way to distinguish between input of 0 and garbage. If a programmer passes it anything that isn't for sure an integer, and they get a 0 back, it's their fault if their code does something unexpected, just like this implementation.
Admin
Libowfat.
Admin
Smells like a hardware design house stumbled into these things called microcontrollers. Newest EE on staff got tossed the project because he had a CS course about six years ago. He still remembered enough coding skilz to try an re-invent the wheel on his own.
As to copy-pasta of standard functions: there lies madness once you start recreating all the dependencies. It's a tough world, you use a standard multiply lib and your code explodes in size (a huge deal if you need to move to a larger micro, especially if the larger part has a different footprint or a higher price). Then you dig into it and realize the standard lib includes a million data formats you'll never need in your application, plus tons of error handling that doesn't apply. Now you're in rewrite land with all its minefields.
captcha: Is a "hamburger" really a "sandwich"?
Admin
"make sure the first character in either a “-” or a digit."
Admin
Isn't the memory violation caused by the first if: if(*s == '-') { negative = TRUE; s++; }
It de-references s, which in case is a null-terminated string de-references null...
Admin
Yeah, I'm not at all convinced by the comments. Yes, bounds checking needs to be done on each character. Yes, this is best done by working from the left. But I really don't see how the empty string would cause a seg fault.
As for fast square root, every architecture which I have seen has some version of an instruction that counts the leading zeros in an integer. Subtract to get the bit length of the integer. Divide by two to get the length of your seed. (If you have a floating point unit to use to get the length of the integer, go ahead & just ask for the square root!)
Admin
And I just spent a good couple of minutes before I realised that it was a while(l) loop, not a while(1) loop.
Admin
Premature damn optimization. This is something a decent compiler should know how to do. How nice that they cared more about one multiply being fast on an infrequently used subroutine* than, you know, correctness?
(*reading in the numbers for atoi is usually going to take much longer than the conversion)
Of course TRWTF is that an embedded system often uses a CPU that doesn't have a "decent" compiler (by modern definitions) and the CPU (8051 is still way more popular than it deserves to be, and PIC is a fucking nightmare) likely has a compiler-unfriendly instruction set. Doubly so if it was more than ten or so years ago, as C compilers (even non-embedded) have gotten a lot better over that time.
I know I've had to pull in my share of "standard" functions, but I at least know to fucking Google it instead of rolling my own every time.
Admin
Wow, that completely shredded the line of code in the quote. Yay, TDWTF <you rock>!
Admin
Who the fuck works backwards for parsing an integer?
Admin
Isn't the memory violation caused by the first if: if(*s == '-') { negative = TRUE; s++; }
No sir, it is not. The string "" is not the NULL string, its a string that only contains the NULL character. It dereferences fine, to yield 0.
Admin
The only way to get a memory violation is to pass NULL, but that is also the behaviour of posix atoi/strtol, at least for clang and gcc.
Admin
Huh. That "atoi()" code looks like something I wrote in about two minutes in an interview for a "leading embedded software company", given the brief that it was to assume sanitised input (mine didn't bother to check for non-numeric chars at all) and be a quick-to-write and simple implementation that didn't use recursion.
I didn't hear back from them, but have heard through the grapevine that they're quite good at that sort of stunt.
Admin
I didn't understand this either. Of course these things can have "standard libraries", just not in the standard way. The fact that you can't just call atoi() != "having to write atoi() from nothing every time you need it"
Admin
Well, I think if you're doing C, most implementations come with a fairly basic C library (my microcontroller's C compiler had a simple one, even had printf!), and you can crib from many freely licensed versions of the C library, including BSD, Android (bionic is a nice small libc, Apache licensed), etc.
If you're dealing with floating point, I think I'd rather crib from an existing libc than re-implement it... And yes, if you lack a hardware multiplier, shift-and-add is faster, otherwise your multiplication is done by a library as well. (Some of the odder link errors I got was due to the linker somehow not including the multiplication and division library... and this was on a modern ARM processor!)
Admin
I think you'll find the fault is caused by
atoi("")
s++ /* s now points past the null byte */ ... strlen(s)
This will scan memory until either it 1) finds a null byte 2) falls off the end of memory or 3) causes a watchdog timer to fail.
Admin
sorry for the formatting. anyway, s is pointing after the null byte and so strlen(s) could do anything.
Admin
A guaranteed memory violation: atoi("-"). s is set to point past the hyphen, then l is set to 0. Each time around the loop, l is decremented and then tested for zero. The first test sees -1, the second sees -2, and eventually it all goes boom.
The ANSI way to do this is to process digits from left to right, stopping at the first non-digit which might be the null terminator. And that requires only one multiply (by ten) per loop.
Admin
This function is VERY sloppy. I just (I was bored) did an atom function and if you take out nice formatting like blank lines and braces on separate lines, it would be about 20 lines, and take into account the many error cases that return zero. I used two local variables (sign holder and result accumulator). Sure if you are doing embedded things you might need to "roll your own", but flubbing it up this much is inexcusable. Optimizing a multiply by 10 is pretty bad too.
Admin
s/atom/atoi/ crazy spellchecker!
Admin
This is a dupe of http://thedailywtf.com/articles/The_Magnitude_of_Ineptitude
Admin
Not quite. The linux manpage for atoi notes that the return value is simply the converted value, and also, that the function does not detect errors. So the return value on a parsing error is actually undefined behaviour, with a 0 value being an implementation detail, not part of the the API contract.
Admin
"If there are typos and grammos in the comments, start being afraid. It indicates a lack of attention to detail. While it is well understood that most people working in computing brag proudly of how dyslexic they are and how only boring old pedants gvie a danm about such stpuid things as speling and making sure the code has"
grammo
spello
"correct, I'm afraid I have found by experience that dyslexic programmers and/or those careless about such details tend to produce the worst code."
When the writer of that comment expects the worst code from him/her self, I shouldn't expect otherwise.
Admin
You also missed all the previous spellos. And the joke.
Admin
10 rounds of Newton-Raphson is enough for a damn good guess at any square root, using the argument as the initial guess (which isn't too bad, as it gets reasonably close on the magnitude). Quadratic convergence is indeed awesome, and square roots are particularly well conditioned. The only pre-flight check you need is for a negative value.
Admin
Indeed, it's a dupe.
Admin
Yes horrendous implementation and yes strlen(s) will be the memory fault, if you are LUCKY enough to have an embedded system with a micro that has memory mamangement hardware to produce that. Assuming of course someone else has not screwed up the MMU setup and handling.
Having worked on micros from 8080,8085, 6809 through to ARM, all are different and sometimes you are stuck for maintenance on an old compiler (for many reasons).
Admin
TRWTF is not implementing atoi as strtol(nptr, NULL, 10);.
Admin
6809, huh? Did you ever get hit by the TEST opcode? (0x14/0x15) For those who don't know, it basically put the CPU into a test mode of reading every address in an infinite loop, which could only be stopped by the RESET line. And it was real fun to debug because I was using an ICE that ran the emulator and target on the same CPU.
That was the day I learned that our hardware watchdog timer would reset on both writes AND reads to its address, when that happened on code running at an unattended site. I don't remember what they did about that little WTF, but at least on one version of our hardware, R/W went into the PAL chip that decoded that.
Admin
For bad initial guess, the first steps of Newton Raphson are approximately division by 2, so if your argument is of the order 2^20, then 10 steps of Newton-Raphson ain't gonna cut it.
Admin
I've copy pasted a LOT of code over the years to avoid wheel reinvention, most often VB, VBS, VBA, C#, Javascript and PHP. I'd say well over 40% of them needed slight Debugging, 30% Heavy Debugging, and 10% throwaways. Of the other 20% that actually work, I usually have to add my own Comments, Variable/Routine name Replace All's, and formatting to make it legible as well.
It's a time-saver, but only if I don't have a clue how to do it in the first place in whatever language I'm using. As I get used to the Language and have my own cloning samples to start with, these code hunts thankfully get less frequent.
POWERSHELL, I almost forgot Powershell. Nasty freaking language and nobody ever comments anything. They all assume you know everything there is to know about Windows API's ahead of time.
Admin
The only possible place for a memory error on atoi("") would be the first if check, but only under the assumption that it triggers a memory error if you try to examine the terminating null byte, which I would hope is not the case as that would basically break everything. Otherwise it evaluates the else if check (which would only trigger a segfault for the same conditions as the first if check) and passing that returns 0.
now atoi("-") does trigger a memory error: checks the first digit, being '-' it sets the negative flag and moves s to the next position strlen("") (presumably) returns 0 p set to position of null p decremented to the position of the '-' rest of do..while loop runs for that iteration l = -1, so continue to next iteration p decremented to the position before the string *p causes segfault (if not during the second time through the loop then eventually it will since l already being negative will increment until p is well outside of valid memory; though I suppose you could get an integer overflow error sometime before then with all of the junk it is adding to t)
Admin
The memory error is triggered in the loop.
strlen("")returns 0, sop = s. Nowp--;brings you to one byte BEFORE the start of s, and the dereference in the line causes a page fault if "" was the only static string in your application, so the data segment is otherwise empty.The actual mistake? Using do {} while(); instead of while(){}
Admin
:wtf:
You save yourself a multiply op, ... after you did a multiply op anyway?
Why not just go through the string in the forward direction, and each time use
?