Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > Hi Ævar, > > On Wed, 5 May 2021, Ævar Arnfjörð Bjarmason wrote: > >> Refactor tr2_dst_try_uds_connect() to avoid a gcc warning[1] that >> appears under -O3 (but not -O2). This makes the build pass under >> DEVELOPER=1 without needing a DEVOPTS=no-error. >> >> This can be reproduced with GCC Debian 8.3.0-6, but not e.g. with >> clang 7.0.1-8+deb10u2. We've had this warning since >> ee4512ed481 (trace2: create new combined trace facility, 2019-02-22). >> >> As noted in [2] this warning happens because the compiler doesn't >> assume that errno must be non-zero after a failed syscall. Let's work >> around it as suggested in that analysis. We now return -1 ourselves on >> error, and save away the value of errno in a variable the caller >> passes in. > > It would probably be a lot nicer if you lead with this insight. I could > imagine, for example, that a oneline like this would be much more helpful > to any reader: > > trace2: do not assume errno != 0 after a failed syscall But that is misleading. My understanding is that this patch is about working around compilers that do not know that a failed syscall means errno would be set to non-zero. Am I mistaken? Otherwise I'd strongly prefer to see a word that hints that this is an otherwise unneeded workaround for comiplers. Your suggested title instead hints that it is wrong to assume that errno will be set to non-zero after a syscall. I do not think that is the message we want to send to our readers.
