Re: removing useless if-before-free tests

[Bruno Haible]

Jim Meyering wrote:
> I agree those should go, along with all of the others in tests/*.c.

I agree about the tests. Tests can afford small slowdowns.

> I'd like to avoid spending time looking for ways to micro-optimize.
> How about if I remove them all, and then restore-with-justification
> the ones for which you think there is a significant performance gain?
> Then, at least, there will be some indication to future reviewers
> why we've kept a seemingly redundant test.

No, I heavily disagree with such an approach. It should be the other way
around: Remove the 'if' only with a written justification to which the
author agrees.

- On this project, I expect everyone to commit complete patches only.
  Saying "here's the first part, I commit it, the second part will come
  later" is OK *only* if the first part does not break anything. Your
  proposal is to break performance for everyone first, and fix it up later.
  No. Not this way.

- Saying "I will break your performance, because I don't have much time,
  then _you_ please fix up what I broke" is simply unfriendly. I don't have
  much more time than you have.

- Code that Paul, I, or Eric wrote has a lot of care and thought in it.
  It is wrong to assume that because there is no comment at a certain place,
  the author did not think about it.

- (A less relevant argument:) Among the occurrences of the pattern in lib/*.c
  that I briefly looked at, the majority appears to be in the case where NULL
  is frequently used, i.e. where the 'if' should be kept for performance.


A different approach would be if you said: "I found some useless 'if's in
these files - Bruno -, in these files - Eric -, in these files - Simon -,
etc. Please look which of them you can remove." This way, you're friendly
towards everyone, you don't break code that is not yours, and you don't
need to spend much time.

Bruno