On 8/8/06, Eric Blake <[EMAIL PROTECTED]> wrote:
> According to Gustavo Rondina on 8/7/2006 8:11 PM:
> > Hello,
> >
> >
> > I was looking the coreutils TODO list and saw that there was still a
> > request
> > for a --total option to df. I have written a patch to implement this
> > feature. It is attached to this message. It worked for me, but I'm
> > no sure if it is fully functional and bug free. This is my first
> > "serious" patch. Any suggestions are welcome.
>
> Thanks for the contribution. However, it is not trivial, so to be
> incorporated, you will need to assign copyright to the FSF. If you
> are willing, we can get that started.
There is no problem for me in assigning copyright to the FSF. I am OK
with the paperwork. First let me improve the code and test it for a
while.
> Can you convince your mailer to attach patches with a text MIME
> type? It makes reviewing a lot easier.
Sorry for that. Next time I will attach it as plain text.
>
> I'm not the maintainer, so I can't say whether your patch will be
> accepted, even if you do complete copyright paperwork. But I did
> notice some problems with just a brief glance.
Thanks for your comments. I will correct the patch and test it
properly and then I will send a new one to the list.
> > ...
> > + {
> > + uintmax_t u100 = total_used.n * 100;
> > + uintmax_t nonroot_total = total_used.n + total_available.n;
> > + pct = u100 / nonroot_total + (u100 % nonroot_total != 0);
> > + }
> > + else
> > + {
> > + /* The calculation cannot be done easily with integer
> > + arithmetic. Fall back on floating point. This can suffer
> > + from minor rounding errors, but doing it exactly requires
> > + multiple precision arithmetic, and it's not worth the
> > + aggravation. */
>
> I'm not sure this is the right approach. Do we really expect systems
> with more than 2^64 bytes of storage?
>
> > + double u = total_used.negative
> > + ? - (double) - total_used.n
> > + : total_used.n;
>
> GNU Coding standards recommend () here:
> double u = (total_used.negative
> ? - (double) - total_used.n
> : total_used.n);
>
> > +
> > + double a = total_available.negative
> > + ? - (double) - total_available.n
> > + : total_available.n;
> > +
> > + double nonroot_total = u + a;
> > + if (nonroot_total)
> > + {
> > + long int lipct = pct = u * 100 / nonroot_total;
> > + double ipct = lipct;
> > +
> > + /* Like `pct = ceil (dpct);', but avoid ceil so that
> > + the math library needn't be linked. */
>
> Why not? We already link it when using a replacement pow(), and the
> library's version is possibly more efficient. Not to mention that I'm
> still not convinced you need to use floating point.
>
This fragments you are referring to are from df itself. They are
related to the percentage calculation and I wasn't pretty sure if I
should create a function or just copy them. I think it is best to
write a function because there is too much redundant code in the way
that I did. Sorry for not mentioning that this was not my code. I will
do it properly now, with comments. I will review this calculation code
and try to improve it, it seems possible.
Again, thanks for your advices.
Regards,
Gustavo
_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
