sudo-project / sudo

Utility to execute a command as another user
https://www.sudo.ws
Other
1.14k stars 209 forks source link

lib/utils: detect failure to generate signals list and names #360

Closed yann-morin-1998 closed 4 months ago

yann-morin-1998 commented 4 months ago

Currently, we generate the signal list and names by running cpp on our header, and piping the result into sed.

However, when cpp fails [0], we do not catch that failure, as the error code of the LHS of a pipe is lost, with the pipe returning the RHS-most return code.

Fix that by introducing two new intermediate rules, each to generate the preprocessed .i files, and use those as dependencies and input to the rule that generates the headers. Those two .i files will be cleaned up by the existing *.i glob.

[0] a failure happens on recent hosts, due to inconsistency with time64_t and large-file support (lines elided and wrapped for readability):

/usr/bin/cpp [...] ./sys_signame.h \
| /usr/bin/sed -e '1,/^int sudo_end_of_headers;/d' -e '/^#/d' > mksigname.h
In file included from /usr/include/features.h:394,
                 from /usr/include/sys/types.h:25,
                 from ./sys_signame.h:4:
/usr/include/features-time64.h:26:5: error: #error "_TIME_BITS=64 is allowed only with _FILE_OFFSET_BITS=64"
   26 | #   error "_TIME_BITS=64 is allowed only with _FILE_OFFSET_BITS=64"
      |     ^~~~~
/usr/bin/gcc [...] ./mksigname.c -o mksigname
In file included from /usr/include/features.h:394,
                 from /usr/include/bits/libc-header-start.h:33,
                 from /usr/include/stdlib.h:26,
                 from ./mksigname.c:27:
/usr/include/features-time64.h:26:5: error: #error "_TIME_BITS=64 is allowed only with _FILE_OFFSET_BITS=64"
   26 | #   error "_TIME_BITS=64 is allowed only with _FILE_OFFSET_BITS=64"
      |     ^~~~~
make[2]: *** [Makefile:263: mksigname] Error 1

In that case, we were lucky that the subsequent gcc call also failed, and for the same reason. That time64_t and lfs issue should be fixed (at least investigated), but that does not mean we should not be more robust when parsing the header either.

yann-morin-1998 commented 4 months ago

@millert

I think you need to remove mksiglist.h.tmp and mksigname.h.tmp when you are done with them. They should also be removed by the clean target.

Thanks for the feedback. 👍

I've gone a little bit further than what you suggested: instead of temp file in the rule, I've made those actual makefile rues, and since they are preprocessed, I renamed the file to .i (which will be cleaned up by the existing *.i glob in the clean rule).