My sentiment exactly.
And by the way, you do have a bug in that code.
My sentiment exactly.
And by the way, you do have a bug in that code.
It correctly tacks a 0 byte on the end if that's what you mean, of course cleaning up the unused space would be outside of the task at hand.
Maybe I'll look like a complete idiot soon if it transpires I've completely missed the 'gotcha' ;)
// esi -> asciiz string
// cl = character to remove
mov edi, esi
cld
loop1:
lodsb // Load the byte from [esi] into al
or al, al
jz alldone
cmp al, cl
je loop1 // Skip this byte, since it's one we want to remove
stosb // Store the byte at [edi] from al
jmp loop1 // Loop around for the next byte
alldone:
xor al, al
stosb // 0 terminate it
The only real behavioral change was that this version would cope with an input of cl=0. My cleaned up version would not cope with cl=0, but would need a sanity check for that.Still, likely my memory is at fault.
But your final solution is clearer than mine. While it uses an extra variable (register), it avoids looking up the same thing twice. So... more efficient and clearer.
BTW: TIL char * a = "hello"; doesn't work in C (I think it doesn't allocate memory). I'm actually happy enough that I could solve this at all, not having used C for 20 years ...old
IMHO That's not a 'bug'. That's a lack of boring gruntwork error checking.
I'm not saying your code is invalid or that it won't work, but 'defensive' programming says:
- assume your input is total garbage
- handle all correct situations correctly
- handle all garbage gracefully or throw an error depending on what the circumstances dictate
I'm also missing the stackframe handling, but as you said, this is the 'meat', that meat needs a bit of scaffolding to work.
How far do you go? Check that the memory isn't mapped as read only? ;) Check that there's actually some memory mapped into that address space?
Obviously you need both skills, but they're pretty related I'd say. If you can solve low level problems you can probably think through any potential issues, invalid inputs, edge cases etc and churn out code to deal with them.
I guess my issue is I don't see what being able to solve this says about someone, apart from "isn't completely terrible at programming".
I did note in my 'answer' that the problem wasn't specified fully.
In this case that wasn't possible, so I assumed the worst, and I think that simply noting that there is a potential problem there would already score you points with the interviewer because they'd realize you spotted that the specification was incomplete, it didn't tell you what to do in case of invalid input.
(I would use some assert()s if a function could cause silent memory corruption; but this particular function either works (valid string), segfaults (NULL) or cannot detect a problem (stray pointer).)
assert(str != NULL)
is being overly generous, I think - if only because a debugger would lead you to the source of the crash immediately.You have not received any specification about the context in which the code operates, therefore the correct course of action is to assume the worse and produce bullet proof code.
If you were operating under constraints in terms of speed that would warrant a more cavalier attitude towards input checking then that would be another thing but since that wasn't specified and there is no context, 'safety first' is the way to approach the issue.
Your assertion would have done the job just fine.
Not all hardware will segfault when writing to NULL, the immutable should segfault and strings that are not null terminated are indeed hard to impossible to check for (pointer wrap..., after a lot of swapping, or a stack overwrite).