Thread: review this code

I came across the following code which is basically just a strcmp implementation.

Code:

int compare_name(const char *p,const char *q)
{
	int d;

	for (;(d=tolower(*p)-tolower(*q))==0;++p,++q) {
		if (!*p) break;
	}
	return d;
}

It never stops to amaze me in how many different ways algorithms are implemented in C

What do you think of this code? it has UB? it works nice?

Originally Posted by Salem

> It never stops to amaze me in how many different ways algorithms are implemented in C
There's certainly a large number of broken ways, especially those which attempt to cram as much as possible into a single line for no good reason.

What happens if p is longer than q, what memory gets accessed?

it seems to work ok there as has been pointed out already. That was the only reason you thought it was broken?

Originally Posted by simpleid

more insight in to programming habits of others is always nice.

Yes, thats why I posted it here. I was also wondering if you guys&girls could see a reason why its implemented in this weird way. the code runs on m68k coldfire

Originally Posted by iMalc

I heard long ago of some compilers that more agressively optimise for loops, over while loops. Perhaps the author of this heard the same thing.
Not sure I even believed it though.

Not to mention that there are ways to do string comparisons for strings much better by using longer items than one char at a time.

And the "for()" keyword can actually be produced as a macro sort of like this (aside from putting the code inside the loop in the macro of course):

Code:

#define FOR(init, condition, update) init; while(condition)  { /* code in loop */; update; }

Since most of this is similar to "while", it's most likely that the optimizing part of the compiler does both for-loops and while-loops.

I find it more likely that the code was more complex to begin with, and got stepwise refined until this was what was left.

--
Mats

Originally Posted by QuestionC

Those are almost, but not quite, the same due to their behavior in the case of 'continue;'.

I remember reading (long ago!) about how some compilers were found to execute

Code:

for(;;) {
   /* ... */
}

faster than

Code:

while (1) {
   /* ... */
}

or variants thereof because the compiler would actually perform an evaluation of testing 1 for truth.

Basically... you just can't assume much anything about how optimization works. Just have faith that it does.

Um, yeah, a compiler that generates code to "evaluate" 1 and not just say "it's a constant that is always true" would be pretty darn poor - but I can believe it. I used some pretty basic compilers on 68K (on Atari/Amiga type machines) many years ago [which may well have something to do with the construct above as it's for a "ColdFire" 68K versiion].

Any modern, decent compiler (such as gcc to name one that is easily available for many architectures) will definitely make no difference between those two cases.

And I don't trust the optimization to do magic - if I want to know that something is optimized the way I expect it to be, I check the compiler output - it's very easy to "fool" a compiler into generating bad code. Most of the time it makes no difference, as most fo what one writes isn't "on the critical path", but if I think it matters, I check it. There are usually ways to fool the compiler back into generating good code if the current code is "bad" - for example use some more variables, using indices instead of updating pointers (or other way around) or simplify the whole piece of code somehow. Using one variable for each thing is a good thing to, don't re-use the same variable for different things - the compiler may not realize that you are not relying on the old value later on, and such, so it will "not do the right thing".

--
Mats

Originally Posted by QuestionC

This is awful behavior. If you've seriously gone to the point of disassembling the program and tweaking the C code to change optimizer behavior... why aren't you just writing the functions in assembly in the first place?

It depends on what you want to achieve. If for example introducing a new variable gives the expected optimization for a large chunk of code, then it's much easier to do that than to use some disassembled C-code to write optimized assembler. And it's much easier to maintain a 15 line piece of C-code than it is to maintain 45 lines of assembler (averaging about 3 lines of assembly for one line of C is probably about right, depending on the complexity of the code of course - lots of indexing or complex expressions can easily push that way higher).

Naturally, when I do this sort of "tweaks to make the compiler do the right thing", I would also add a comment to say so - otherwise it's just obscuring things.

By the way, both MS and GCC have switches to output the assembly (with C-code attached in more recent versions), so you don't need to DISASSEMBLE anything.

Most of the time, the optimizer does a good job, so it's rare that I find myself doing this - but it has happened. For one thing, most of the code isn't performance critical anyways, right? So unless it's either KNOWN for one reason or another to be part of the performance critical code, or you find it during profiling, no point to touch it.

--
Mats