minor C-style fixes #14

Closed
wants to merge 2 commits into from

2 participants

@rck

Hi muennich,

I changed two little things I consider worth merging:

*) I used the already defined EXIT_SUCCESS / EXIT_FAILURE macros for exit codes
*) I think it is bad style to use numeric return values in a boolean context (e.g. strcmp does not return a bool, but the return value is used as one => !not_a_bool in if-statements. Therefore, I introduced a STREQ macro in utils.h. I think the result is a) more readable and b) STREQ returns a bool. I think "STREQ(foo, bar)" is nicer to read than "!strcmp(foo, bar)".

hope you like and merge it,
best regards, rck

@muennich
Owner

Thanks. Directly merged 3a81af4. Slightly changed the STREQ macro. Using "sys/types.h" for ssize_t.

@muennich muennich closed this Sep 26, 2011
@rck

Hi muennich,

May I ask you why you did not take the strcmp(a,b) == 0 version? To get things straight, it is your project (a very nice one btw), but I am curios why people write code the way they do. maybe because I am teaching C at the university myself...

I still think using !strcmp (or more genaral using the return value of something that is not boolean in a boolean context) is bad style. What is the reason for you?

Some opinions on that topic:

openbsd style [1]:

Don't use `!' for tests unless it's a boolean, i.e., use...

c-faq (imo a very good reference) [2]:

Notice that strcmp does not return a Boolean, true/false, zero/nonzero answer, so it's not a good idea to write something like: if(strcmp(string3, string4))

sysprog (course @ TU Vienna, sorry it is german) [3]:

Logische Werte sind mit logischen Operatoren zu behandeln, numerische Werte mit arithmetischen Operatoren (z.B. auf 2 Strings auf Gleichheit testen: nicht !strcmp(...) verwenden, sondern strcmp(...) == 0 !).

again, I do not blame you in any sense, I am just interested.

[1] http://www.openbsd.org/cgi-bin/man.cgi?query=style
[2] http://c-faq.com/~scs/cclass/notes/sx8.html
[3] http://ti.tuwien.ac.at/rts/teaching/courses/systems_programming/Richtlinien

best regards, rck

@muennich
Owner

You convinced me. I will change this boolean test thingy in the near future. But this also requires rewriting all the NULL-pointer checks, because they also use the logical negation operator instead of "== NULL".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment