Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-08 Thread Paolo Bonzini
I disagree. The = change inserts a conditional branch into the control flow, with the potential to save a single memory access. I count ca. 2 CPU cycles for a memory access and ca. 8 CPU cycles for a conditional jump, therefore I would say that the change slows down the program a bit.

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-04 Thread Jim Meyering
Bruno Haible [EMAIL PROTECTED] wrote: Jim Meyering wrote: Other opinions welcome. I mostly agree with Eric here: gnulib's substitute does not guarantee that values stored in a 'bool' are either 0 or 1, therefore the code that creates 'bool' values must guarantee it. The question is how

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-04 Thread Ralf Wildenhues
Hello, Bruno Haible [EMAIL PROTECTED] wrote: Jim Meyering wrote: Some of the changes ( = ) are unconditional improvements, imho. I disagree. The = change inserts a conditional branch into the control flow, with the potential to save a single memory access. I count ca. 2 CPU

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Kamil Dudka
On Tuesday 02 September 2008 16:01:13 you wrote: Would you please amend/squash the patch below into your patch and adjust the line lengths of the log message to be = 72, so that the generated ChangeLog lines don't wrap? No problem, here is (I hope) complete patch. Also thanks for the regexp, it

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Andreas Schwab
Kamil Dudka [EMAIL PROTECTED] writes: @@ -247,6 +254,41 @@ df_readable (bool negative, uintmax_t n, char *buf, } } +/* Logical equivalence */ +#define LOG_EQ(a, b) (!(a) == !(b)) + +/* Add integral value while using uintmax_t for value part and separate + negation flag. It adds

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Kamil Dudka
On Wednesday 03 September 2008 11:03:22 you wrote: Kamil Dudka [EMAIL PROTECTED] writes: Since both arguments are already bool I see no need for LOG_EQ (it's the only use anyway). If you are using type bool, there is no guarantee there will be bool (0/1) value inside. It ca be (and mostly is)

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Andreas Schwab
Kamil Dudka [EMAIL PROTECTED] writes: On Wednesday 03 September 2008 11:03:22 you wrote: Kamil Dudka [EMAIL PROTECTED] writes: Since both arguments are already bool I see no need for LOG_EQ (it's the only use anyway). If you are using type bool, there is no guarantee there will be bool (0/1)

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Jim Meyering
Andreas Schwab [EMAIL PROTECTED] wrote: Kamil Dudka [EMAIL PROTECTED] writes: On Wednesday 03 September 2008 11:03:22 you wrote: Kamil Dudka [EMAIL PROTECTED] writes: Since both arguments are already bool I see no need for LOG_EQ (it's the only use anyway). If you are using type bool, there

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Kamil Dudka
On Wednesday 03 September 2008 11:18:37 you wrote: Kamil Dudka [EMAIL PROTECTED] writes: On Wednesday 03 September 2008 11:03:22 you wrote: Kamil Dudka [EMAIL PROTECTED] writes: Since both arguments are already bool I see no need for LOG_EQ (it's the only use anyway). If you are using

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Andreas Schwab
Jim Meyering [EMAIL PROTECTED] writes: Andreas Schwab [EMAIL PROTECTED] wrote: Kamil Dudka [EMAIL PROTECTED] writes: On Wednesday 03 September 2008 11:03:22 you wrote: Kamil Dudka [EMAIL PROTECTED] writes: Since both arguments are already bool I see no need for LOG_EQ (it's the only use

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Jim Meyering
Andreas Schwab [EMAIL PROTECTED] wrote: Jim Meyering [EMAIL PROTECTED] writes: Andreas Schwab [EMAIL PROTECTED] wrote: Kamil Dudka [EMAIL PROTECTED] writes: On Wednesday 03 September 2008 11:03:22 you wrote: Kamil Dudka [EMAIL PROTECTED] writes: Since both arguments are already bool I see no

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Andreas Schwab
Jim Meyering [EMAIL PROTECTED] writes: I presume you're referring to uses of bool variables like these (there are many more): I'm referring to the use of the very same variables that are used in the patch. If those are not pure boolean then you have a bug anyway. Andreas. -- Andreas

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Jim Meyering
Andreas Schwab [EMAIL PROTECTED] wrote: Jim Meyering [EMAIL PROTECTED] writes: I presume you're referring to uses of bool variables like these (there are many more): I'm referring to the use of the very same variables that are used in the patch. If those are not pure boolean then you have

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Andreas Schwab
Jim Meyering [EMAIL PROTECTED] writes: Andreas Schwab [EMAIL PROTECTED] wrote: Jim Meyering [EMAIL PROTECTED] writes: I presume you're referring to uses of bool variables like these (there are many more): I'm referring to the use of the very same variables that are used in the patch. If

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Jim Meyering
Kamil Dudka [EMAIL PROTECTED] wrote: On Tuesday 02 September 2008 16:01:13 you wrote: Would you please amend/squash the patch below into your patch and adjust the line lengths of the log message to be = 72, so that the generated ChangeLog lines don't wrap? No problem, here is (I hope)

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Eric Blake
I'm referring to the use of the very same variables that are used in the patch. If those are not pure boolean then you have a bug anyway. Here are some of the changes needed to protect against the substandard bool problem we're talking about. Some of the changes ( = ) are

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-03 Thread Jim Meyering
[EMAIL PROTECTED] (Eric Blake) wrote: I'm referring to the use of the very same variables that are used in the patch. If those are not pure boolean then you have a bug anyway. Here are some of the changes needed to protect against the substandard bool problem we're talking about. Some of

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Andreas Schwab
Kamil Dudka [EMAIL PROTECTED] writes: @@ -296,7 +338,9 @@ show_dev (char const *disk, char const *mount_point, if (!stat_file) stat_file = mount_point ? mount_point : disk; - if (get_fs_usage (stat_file, disk, fsu)) + if (force_fsu) +memcpy(fsu, force_fsu, sizeof(struct

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Kamil Dudka
On Tuesday 02 September 2008 10:55:05 you wrote: fsu = *force_fsu; I was not sure, if all supported compiler take this. bzero is non-standard, and static variables are always initialized. I have already heard about it. But inode_format, show_all_fs, show_listed_fs, ... are initialized

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Andreas Schwab
Kamil Dudka [EMAIL PROTECTED] writes: On Tuesday 02 September 2008 10:55:05 you wrote: fsu = *force_fsu; I was not sure, if all supported compiler take this. Structure assignment is part of C since even before KR2. http://c-faq.com/struct/firstclass.html Andreas. -- Andreas Schwab,

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Jim Meyering
Kamil Dudka [EMAIL PROTECTED] wrote: ... New patch in attachment. Hi Kamil, Thanks for working on this! You'll want to print totals with --inodes (-i), too. Please adjust formatting to use spaces before each open parenthesis and drop the short-named -c option. There is a strong disincentive

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Andreas Schwab
Jim Meyering [EMAIL PROTECTED] writes: Kamil Dudka [EMAIL PROTECTED] wrote: +#define LOG_EQ(a,b) (((a)(b))||(!(a)!(b))) This can be written more simply as !((a) ^ (b)) Only if the operands are already boolean, and then you can just use a == b. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Jim Meyering
Andreas Schwab [EMAIL PROTECTED] wrote: Jim Meyering [EMAIL PROTECTED] writes: Kamil Dudka [EMAIL PROTECTED] wrote: +#define LOG_EQ(a,b) (((a)(b))||(!(a)!(b))) This can be written more simply as !((a) ^ (b)) Only if the operands are already boolean, and then you can just use a == b. Oh.

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread James Youngman
On Tue, Sep 2, 2008 at 12:06 PM, Jim Meyering [EMAIL PROTECTED] wrote: Andreas Schwab [EMAIL PROTECTED] wrote: Jim Meyering [EMAIL PROTECTED] writes: Kamil Dudka [EMAIL PROTECTED] wrote: +#define LOG_EQ(a,b) (((a)(b))||(!(a)!(b))) This can be written more simply as !((a) ^ (b)) Only if the

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Jim Meyering
James Youngman [EMAIL PROTECTED] wrote: On Tue, Sep 2, 2008 at 12:06 PM, Jim Meyering [EMAIL PROTECTED] wrote: Andreas Schwab [EMAIL PROTECTED] wrote: Jim Meyering [EMAIL PROTECTED] writes: Kamil Dudka [EMAIL PROTECTED] wrote: +#define LOG_EQ(a,b) (((a)(b))||(!(a)!(b))) This can be written

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Kamil Dudka
On Tuesday 02 September 2008 11:52:57 you wrote: You'll want to print totals with --inodes (-i), too. Good point, fixed... Please adjust formatting to use spaces before each open parenthesis Sorry for this detail, I always forget. Note that this is not acceptable for macro definition with

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Jim Meyering
Kamil Dudka [EMAIL PROTECTED] wrote: On Tuesday 02 September 2008 11:52:57 you wrote: You'll want to print totals with --inodes (-i), too. Good point, fixed... Please adjust formatting to use spaces before each open parenthesis Sorry for this detail, I always forget. Note that this is not

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Andreas Schwab
Jim Meyering [EMAIL PROTECTED] writes: diff --git a/tests/df/total b/tests/df/total index be4bc19..5398deb 100755 --- a/tests/df/total +++ b/tests/df/total @@ -29,12 +29,12 @@ fail=0 umask 22 df tmp || fail=1 -grep ^total tmp fail=1 +grep '^total' tmp fail=1 This will fail if you

Re: [PATCH] df: new option: --total (-c) to produce grand total (in the same way as du)

2008-09-02 Thread Jim Meyering
Andreas Schwab [EMAIL PROTECTED] wrote: Jim Meyering [EMAIL PROTECTED] writes: diff --git a/tests/df/total b/tests/df/total index be4bc19..5398deb 100755 --- a/tests/df/total +++ b/tests/df/total @@ -29,12 +29,12 @@ fail=0 umask 22 df tmp || fail=1 -grep ^total tmp fail=1 +grep