On Wed, 8 Mar 2017, Michael J. Tubby B.Sc. MIET wrote:
> Phil Pennock has just posted the availability of 4.89 release to
> exim-announce.
>
> I've downloaded and compiled on Ubuntu 14.04.5 and get one warning during
> compilation:
>
> gcc drtables.c
> gcc enq.c
> gcc exim.c
> exim.c: In function ‘usr1_handler’:
> exim.c:235:1: warning: ignoring return value of ‘write’, declared with
> attribute warn_unused_result [-Wunused-result]
> (void)write(fd, process_info, process_info_len);
> ^
> gcc expand.c
> gcc filter.c
> gcc filtertest.c
> gcc globals.c
> gcc dkim.c
> and believing that cleanliness is next to godliness have fixed the warning by
> removing the (void) cast over the write function and assigning the return
> value to variable 'sz'.
>
> My version of usr1_handler() is provided below in 1TBS style and allows Exim
> to compile without warning on Ubuntu systems:
>
Which version of gcc are you using ?
On my Scientific Linux 6.8 machine, both
gcc 4.9.2 20150212 (Red Hat 4.9.2-6)
and
gcc 5.3.1 20160406 (Red Hat 5.3.1-6)
are silent with
gcc -Wunused-result -c -O -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I. exim.c
for both the original version and your version with sz.
However,
gcc -Wall -Wunused-result -c -O -D_FILE_OFFSET_BITS=64 \
-D_LARGEFILE_SOURCE -I. exim.sz.c
complains:
exim.sz.c: In function usr1_handler:
exim.sz.c:212:11: warning: variable sz set but not used [-Wunused-but-set-variable]
ssize_t sz;
^
for your version (as well as several other warnings).
For the original code, these compilers with -Wall -Wunused-result are
happy with the original version of usr1_handler.
One of the other warnings that -Wall (and -Wunused-but-set-variable) give
for this file is
exim.c: In function main:
exim.c:4128:7: warning: variable dummy set but not used [-Wunused-but-set-variable]
int dummy;
.. so at least your change makes the file more self-consistent.
Both versions of the code ignore the return value, so I think it is
correct that there should be a warning until/unless we handle
any unexpected return values from write().
Since this is a signal handling routine, ignoring the return value from
write *might* actually be safer ...
[
In another email Jeremy Harris said
... and you just _know_ that the <swearword> static analysers
next year will be telling you that you assigned this variable
and then never used it.
Sorry, Jeremy, you don't even get a year's grace :-(
]
> static void usr1_handler(int sig)
> {
> int fd;
> ssize_t sz;
>
> os_restarting_signal(sig, usr1_handler);
>
> fd = Uopen(process_log_path, O_APPEND|O_WRONLY, LOG_MODE);
>
> if (fd < 0) {
> /* If we are already running as the Exim user, try to create it in the
> current process (assuming spool_directory exists). Otherwise, if we are
> root, do the creation in an exim:exim subprocess. */
>
> int euid = geteuid();
> if (euid == exim_uid)
> fd = Uopen(process_log_path, O_CREAT|O_APPEND|O_WRONLY, LOG_MODE);
> else if (euid == root_uid)
> fd = log_create_as_exim(process_log_path);
> }
>
> /* If we are neither exim nor root, or if we failed to create the log file,
> give up. There is not much useful we can do with errors, since we don't
> want
> to disrupt whatever is going on outside the signal handler. */
>
> if (fd < 0)
> return;
>
> sz = write(fd, process_info, process_info_len);
> close(fd);
> }
>
>
>
> Mike
>
>
> --
> ## List details at https://lists.exim.org/mailman/listinfo/exim-users
> ## Exim details at http://www.exim.org/
> ## Please use the Wiki with this list - http://wiki.exim.org/
>
