Re: [PATCH 3/5] convert: Use the enum constant SAFE_CRLF_FALSE.

"Henrik Grubbström (Grubba)"  <grubba@grubba.org> writes:

I have a feeling that you are fixing a wrong problem.

If anything, I personally think that the original that passes 0 makes it easier to read the callers, and I suspect that the primary reason why it is so is because SAFE_CRLF_FALSE is grossly misnamed. X_FALSE makes the reader wonder "perhaps I can change it to X_TRUE and make something interesting happen?" but there of course is no SAFE_CRLF_TRUE.

Look at the callee that _ought_ to use the enum constant but doesn't:

    static void check_safe_crlf(const char *path, int action,
                                struct text_stat *stats, enum safe_crlf checksafe)
    {
            if (!checksafe)
                    return;
    ...

The "checksafe" is used to specify "what should be done if it turns out to be unsafe after inspection", and passing 0 is "won't do anything, so there is no point to even check". Both callers and the callee _know_ that 0 means "don't bother", and thus this callee doesn't bother.

If the constant were renamed from SAFE_CRLF_FALSE to something a bit more sensible (perhaps SAFE_CRLF_NOWARN? SAFE_CRLF_NOOP?), then it might make sense to replace these 0 with symbolic constants and argue that such a change makes the code easier to read.