tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Why no SIGINFO?



> diff -u -r1.57 cp.c
> --- cp.c      18 Aug 2011 08:11:58 -0000      1.57
> +++ cp.c      4 Jan 2012 05:42:59 -0000
> @@ -74,6 +74,7 @@
> #include <stdio.h>
> #include <string.h>
> #include <unistd.h>
> +#include <signal.h>

Something seems to have stripped the leading space from untouched
lines.  (It did not, however, also strip the tab from lines, later in
your patch but which I haven't quoted, that should begin with a space
(from diff) and a tab (from the source).)  You might want to check your
MUA.

> +                             if (pinfo) {
> +                                     snprintf(infobuf, 160,
> +                                     "%s => %s %zu/%zu bytes %d%% written\n",
> +                                     entp->fts_path, to.p_path, 
> (size_t)ptotal,
> +                                     (size_t)fs->st_size, (int)(100.0 * 
> ptotal / fs->st_size));
> +                                     (void)write(STDERR_FILENO, infobuf, 
> strlen(infobuf));
> +                                     pinfo = 0;
> +                             }

I'm not sure whether I'd prefer to clear pinfo before printing.
There's a race either way; a SIGINFO arriving between the print and the
assignment will misbehave slightly as compared to the naïve idealized
semantics.  In this case, I doubt it makes much difference, but in
cases where it really matters, such as synchronizing the top and bottom
halves of a device driver, it's usually better to clear the flag before
doing the work, so it is arguably a good habit to get into.

Slightly more seriously, but still not very important, you do not need
to cast ptotal to size_t, since it's already delcared as being size_t.

Why snprintf and write rather than fprintf (and possibly fflush)?
You're not executing inside a signal handler at this point (are you? at
least not the SIGINFO handler), so the only difference I can see is the
length clipping, and I think that's actually undesirable.
(Furthermore, if you do stick with snprintf, I'd say the 160 should
actually be sizeof(infobuf).)

I'd recommend either putting parens around 100.0 * ptotal or moving the
float to the middle position (ie, ptotal * 100.0 / fs->st_size).  What
you have written will fail if the division is done first.  While I
think * and / associate left-to-right, I very much prefer to write such
code defensively (indeed, I parenthesize even when doing * inside +,
which is arguably overdoing the defensive parenthesization).  Putting
the float in the middle forces both operations to be done as floating
arithmetic regardless of which way the operators bind.

/~\ The ASCII                             Mouse
\ / Ribbon Campaign
 X  Against HTML                mouse%rodents-montreal.org@localhost
/ \ Email!           7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B


Home | Main Index | Thread Index | Old Index