On Thu, Oct 19, 2023 at 10:52:11AM -0700, Junio C Hamano wrote:
>
> Also some of them might better be a BUG(), instead of die(_()).
>
> > * crlf_action in convert.c:
> >
> > warning(_("illegal crlf_action %d"), (int)crlf_action);
>
> [jch: cc'ed Torsten for area expertise].
>
> For example, can convert.c::output_eol() be called with an illegal
> crlf_action that is not covered by the switch() statement due to
> data error, not a programming error? From my quick scan, it looks
> like that the error should never happen no matter what end-user
> mistakes (e.g., misspelt attribute and configuration variable names
> in their files) are fed to convert_attrs(), and can come only from a
> bug in that function (e.g., long and convoluted if/else cascade fails
> to assign any value to ca->crlf_action and leaves an undefined and
> "illegal" value there).
The switch case covers all 8 values of "enum crlf_action",
and removing these 2 lines
- warning("Illegal crlf_action %d\n", (int)crlf_action);
- return core_eol;
does still compile without a compiler warning.
So yes, a BUG is more appropriate here.
I hopefully find some time to send a patch the next days.
>
> Thanks.
>
