Re: [PATCH] Mention the new join option --check-order

2008-02-16 Thread Pádraig Brady
James Youngman wrote:
> +  join now has a --check-order option which causes join to verify that
> +  the input files are indeed sorted.  The option --nocheck-sorted
> +  turns the check off (the check is currently off by default).

I'm not sure it's worth adding this option.
If you know to use it, then you already know the input should be sorted?
what am I missing?

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Mention the new join option --check-order

2008-02-17 Thread Pádraig Brady
James Youngman wrote:
> On Feb 17, 2008 12:16 AM, Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> James Youngman wrote:
>>> +  join now has a --check-order option which causes join to verify that
>>> +  the input files are indeed sorted.  The option --nocheck-sorted
>>> +  turns the check off (the check is currently off by default).
>> I'm not sure it's worth adding this option.
>> If you know to use it, then you already know the input should be sorted?
>> what am I missing?
> 
> My original intent was to make it the default, but it turns out that
> the coreutils manual documents a "GNU extension" allowing the input
> not to be sorted, as long as identical join fields are presented on
> each line.

That condition can be easily checked for right?
So you print the warning if you detect unsorted input,
and the above condition is not true.

Also I can't think of any other reason you would input
unsorted keys into `join`, so it would be great if
we could add this functionality with no options at all.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] better diagnostics for seq

2008-02-18 Thread Pádraig Brady
Steven Schubiger wrote:
> Attached is a patch that enhances seq's diagnostics. If you agree
> that this is the right way to go, I'll amend other files (ChangeLog,
> etc.) as needed.

Seems sensible. This is what I get on a reasonably recent tree:

$ ./seq -f% 1
./seq: memory exhausted

Note however that on gutsy I get the expected:

$ seq -f% 1
seq: invalid format string: `%'
Try `seq --help' for more information.

So perhaps you could use the existing patch/logic from gutsy?

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Implement join --check-order and --nocheck-order.

2008-02-19 Thread Pádraig Brady
James Youngman wrote:
> This is a consolidated patch including all the previous changes,
> implementing order checking in join by default.  "make distcheck"
> works (if I move distcheck-hook from Makefile.am to GNUmakefile).

While I like the idea, I'm a little worried about the implementation.
You seem to duplicate the buffers rather than the pointers.
Do you really need to do the extra malloc() + memcpy() per line read?

Could you compare the performance of before and after (with LANG=C).

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#467378: coreutils: Please include a program to truncate files

2008-02-25 Thread Pádraig Brady
Jim Meyering wrote:
> Russell Coker <[EMAIL PROTECTED]> wrote:
>> If I have a file that is 2G in size but wish to discard the last 1G of data
>> then there seems to be no program available to do this.
>>
>> I think it would be ideal to have a program as part of coreutils that allows
>> you to resize a file.  If the new length is longer than the old length then
>> it would either write zeros to the end or extend the file (with a hole)
>> via the truncate() system call according to the wish of the user.  If the new
>> length is shorter then it would just call truncate().
>>
>> I would be happy to contribute the code for this.  This will require some
>> discussion with upstream, but it seemed best to start the discussion here.
> 
> That would be nice.
> 
> If the mythical miscutils project were more concrete,
> I'd say this tool belongs there.

I'm planning to push code to this very "project" very soon.
As previously discussed I'm going to push this to
the misc-utils directory of the util-linux-ng project.
We can split that directory as a new project in future
if ever deemed necessary.

> I'm not 100% sure it should be written in C, especially
> since I wrote it in Perl:

And here's a shell version:
http://www.pixelbeat.org/scripts/truncate
I'm probably going to (re)write all "miscutils" in C though.

> Russell, if you want to write a C version and do the usual
> complete job (update coreutils.texi, NEWS, etc. and add tests),
> I'd probably go for it.  The only reason not to would be if
> miscutils is now viable, and even then...
> One advantage of using C/coreutils/gnulib
> is that you'd get k, M, G, ... byte-count suffixes for free.

Note I already got util-linux-ng using the "coreutils" suffixes,
so this should be a good fit.

Russell if you want to push code to me it's fine,
but I was going to work on this tool anyway.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: argve0, psfool

2008-02-25 Thread Pádraig Brady
Andreas Schwab wrote:
> Andreas Schwab <[EMAIL PROTECTED]> writes:
> 
>> Brian Dessent <[EMAIL PROTECTED]> writes:
>>
>>> "Daniel C. Bastos" wrote:
>>>
 I always miss these two programs on every system I meet. argv0 is very
 handy when dealing with programs that care about argv[0] and psfool is
 essential when giving out passwords through the command line. I figure
 these two should be in coreutils.
>>> perl -e 'exec { "real" } "fake", "arg1", "arg2"'
>> bash -c 'exec -a "$0" "$@"' real fake arg1 arg2
> 
> That should of course be:
> 
> bash -c 'exec -a "$0" "$@"' fake real arg1 arg2

I think that should be?

bash -c 'exec -a "$@"' fake real arg1 arg2

Anyway, why would one use an argve0 tool rather than use a symlink?
A real example would be great.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#467378: coreutils: Please include a program to truncate files

2008-02-26 Thread Pádraig Brady
Jim Meyering wrote:
> Paul Eggert <[EMAIL PROTECTED]> wrote:
>> Jim Meyering <[EMAIL PROTECTED]> writes:
>>> If you don't mind truncating first, how about this?
>>>
>>> true > /var/spool/whatever/foo
>>> dd bs=1 seek=2G of=/var/spool/whatever/foo < /dev/null
>> Also, the latter command works even if the former command is omitted.
>> That is, by itself, that invocation of dd resizes
>> /var/spool/whatever/foo to 2 GiB, discarding or extending the file as
>> needed, which is what the original request asked for.
> 
> Hi Paul,
> 
> That depends on your definition of "works".
> If you don't mind retaining the first 2GiB of content in
> a preexisting output file, then it works fine.  But the initial
> truncation is required if you want to be sure it's a big sparse
> file with nothing but NUL bytes.
> 
> Try this:
> 
> echo foo > k
> dd bs=1 seek=2G of=k < /dev/null
> head -c4 k
> 
> and note that it prints "foo\n".

That's how I would expect it to work,
and is how I'm currently implementing it.
Given the name "truncate" I think this
operation should be obvious to users.

Pádraig.

p.s. I'm going to push the "timeout" and "truncate"
utils to util-linux-ng/misc-utils/  via Karel Zak
within the next couple of days.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#467378: coreutils: Please include a program to truncate files

2008-02-26 Thread Pádraig Brady
Russell Coker wrote:
> On Tuesday 26 February 2008 21:03, Pádraig Brady <[EMAIL PROTECTED]> wrote:
>>> That depends on your definition of "works".
>>> If you don't mind retaining the first 2GiB of content in
>>> a preexisting output file, then it works fine.  But the initial
>>> truncation is required if you want to be sure it's a big sparse
>>> file with nothing but NUL bytes.
>>>
>>> Try this:
>>>
>>> echo foo > k
>>> dd bs=1 seek=2G of=k < /dev/null
>>> head -c4 k
>>>
>>> and note that it prints "foo\n".
>> That's how I would expect it to work,
>> and is how I'm currently implementing it.
>> Given the name "truncate" I think this
>> operation should be obvious to users.
> 
> What exactly do you mean by "how I expect it to work", do you mean "reducing 
> the file size" or "removing all content and extending it to size N"?  The 
> latter is not truncation, it's replacement.

Exactly. I expect it to work as per Jim's example above.
I.E. truncate rather than replace.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bugreport 'sort'

2008-03-04 Thread Pádraig Brady
Heijnen wrote:
> Hi,
> 
> I want to report a bug in "sort" utility.

For the umpteenth time this locale "bug" has been reported,
because the default behaviour is unexpected and fixing it requires
knowledge of how locales and environment variables work.

I wonder should we flag the common case on exit with something like:

if (only_ascii_read && LC_COLLATE!="C" && !LC_ALL) {
   fprintf(stderr, "Warning, sorting ASCII data in non C locale\n");
   fprintf(stderr, "Please be explicitly describe your data by doing either\n");
   fprintf(stderr, "`LC_ALL=C sort` or `LC_ALL=$LANG sort`\n"); /* sh syntax 
for brevity */
}

Pádraig


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bugreport 'sort'

2008-03-04 Thread Pádraig Brady
Eric Blake wrote:
> Pádraig Brady  draigBrady.com> writes:
> 
>> I wonder should we flag the common case on exit with something like:
>>
>> if (only_ascii_read && LC_COLLATE!="C" && !LC_ALL) {
>>fprintf(stderr, "Warning, sorting ASCII data in non C locale\n");
> 
> Nice thought.  But it would violate POSIX if done by default

POSIX says you can't warn users about potential problems?

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bugreport 'sort'

2008-03-04 Thread Pádraig Brady
Eric Blake wrote:
> Pádraig Brady  draigBrady.com> writes:
> 
>>> Nice thought.  But it would violate POSIX if done by default
>> POSIX says you can't warn users about potential problems?
> 
> One person's bug is another person's feature.  The entire reason that 
> LC_COLLATE exists is because some people INTENTIONALLY want to sort files in 
> other than the C locale order, when they know what they are doing.

As a related issue, why do we indicate in the man page and FAQ to explicitly
set the order use LC_ALL rather than LC_COLLATE ?
I was just using that as a quick example of heuristics one
might be able to use to determine if a user was being explicit or not.

> POSIX 
> pretty much states that if an application prints a message to stderr, it 
> should 
> exit with non-zero status, unless explicitly mentioned otherwise in the POSIX 
> requirements for that application.

So one can't print warning messages, hmm.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Coreutils Command "sort" Needs Manual Clarity

2008-03-04 Thread Pádraig Brady
Tony Kocurko wrote:
> Hi, All...
> 
>  It might be that the man page for sort needs some
> polishing. I discovered that in the "-k F[.C],F[.C]"
> command line argument pair, for fields beyond the
> first, the first character appears to be the first
> delimiter character prior to the field.

That (confusing default) is documented in the man page:

"If neither -t nor -b is in effect, characters in a field
are counted from the beginning of the preceding whitespace."

> # cat SortProblem1
> ABC 010201
> DEF 010202
> GHI 020203
> JKL 010204
> MNO 010208
> PQR 010301
> STU 010302
> 
> [First I thought it should be:]
> sort -k 2.5,2.6n -k 2.1,2.2n -k 2.3,2.4n
> [then I realised it should be:]
> sort -k 2.6,2.7n -k 2.2,2.3n -k 2.4,2.5n

Since all fields are the same type you can
simplify it further to:

sort -bn -k2.5,2.6 -k2.1,2.2 -k2.3,2.4

In fact because this is fixed with data you
don't need to convert to numeric for comparison at all,
and you know this will only be ASCII data so
you can tell sort to use the fast "C" data comparison:

LC_ALL=C sort -b -k2.5,2.6 -k2.1,2.2 -k2.3,2.4

Note a subtlety with specifying the global -b option
is also documented in the man page:

"OPTS is one or more single-letter ordering options,
which override global ordering options for that key."

That means that if you do need to change the sort type
for a specific field, then you will need to also specify the
b option again to both positions.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#469525: coreutils: [wc] RFC: Add option --no-filename

2008-03-06 Thread Pádraig Brady
Jari Aalto wrote:
> Package: coreutils
> Version: 6.10-3
> Severity: wishlist
> 
> 
> Description:
> 
>   $ wc -l README
>   200 README
> 
> This is fine for interactive use, but not convenient in shell scripts,

Options are bad because of the extra complexity presented to users.
Currently wc only suppresses the "total" if only one file is passed.
I wonder could we go one step further and suppress the filename
also like grep does when passed a single file?
Note existing scripts that do the following would continue to work:

lines=`wc -l file | cut -d ' ' -f1`

It's probably safer though to not change the current operation,
considering that the functionality can be satisfied using the above,
or the following:

lines=`wc -l http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature Request: du --dir-with-slash

2008-03-06 Thread Pádraig Brady
Thomas Guettler wrote:
> Hi,
> 
> I use "du -ax / | sort -rn > /var/tmp/du-`date --iso`" to get the sorted
> 
> Unfortunately you can't see a difference between a directory and a file
> in the output. It would be nice if the directories would be added by a
> slash.
> 
> This way you can see if a line is a directory or not. This is even more
> important
> if you want to use the output for a script.
> 
> I suggest 'du --dir-with-slash' or 'du --trailing-slash' as parameter.

If we were going to do that we should do it unconditionally.
I can't see how it would break existing scripts.
POSIX can't disallow that can it!

As an alternative try:

find / -xdev -printf "%k\t%p%y\n" |
sed 's/d$/\//;t; s/.$//;' |
LC_ALL=C sort -rn -k1,1

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


[PATCH] Add misc-utils/timeout utility

2008-03-07 Thread Pádraig Brady
Hi Karel,

I've attached a patch to add the "timeout" utility
which was previously discussed on coreutils and util-linux-ng lists.

I've tested this in a lot of situations and have
noted the following caveats:

If you start a command in the background, which reads from the tty
and so is immediately sent SIGTTIN to stop, then the timeout
process will ignore this so it can timeout the command as expected.
This can be seen with `timeout 10 dd&` for example.
However if one brings this group to the foreground with the `fg`
command before the timer expires, the command will remain
in the sTop state as the shell doesn't send a SIGCONT
because the timeout process (group leader) is already running.
To get the command running again one can Ctrl-Z, and do fg again.
Note one can Ctrl-C the whole job when in this state.
I think this could be fixed but I'm not sure the extra
complication is justified for this scenario.

If one does Ctrl-Z then the timeout (and command) will be stopped
but the alarm timer keeps running, so that once the timeout process is
resumed it will exit if the timer has expired at this time.

If specify -s9 to kill timeout jobs with, then
exit code will not be ETIMEDOUT, just standard killed exit code.

cheers,
Pádraig.

p.s. I'll send on the misc-utils/truncate utility next
Should be a _lot_ easier to test than this one.
>From 95f14d6a09894bfcba034a281bccd46b9da05f1d Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Fri, 7 Mar 2008 10:48:34 +
Subject: [PATCH] Add the timeout utility


Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 misc-utils/timeout.1 |   31 
 misc-utils/timeout.c |  188 ++
 2 files changed, 219 insertions(+), 0 deletions(-)
 create mode 100644 misc-utils/timeout.1
 create mode 100644 misc-utils/timeout.c

diff --git a/misc-utils/timeout.1 b/misc-utils/timeout.1
new file mode 100644
index 000..1a60791
--- /dev/null
+++ b/misc-utils/timeout.1
@@ -0,0 +1,31 @@
+.\" Copyright 2008 Padraig Brady ([EMAIL PROTECTED])
+.\" May be distributed under the GNU General Public License
+.TH TIMEOUT 1 "7 March 2008" "Linux Utilities" "Linux Programmer's Manual"
+.SH NAME
+timeout \- Start a command, and kill it if the timeout expires
+.SH SYNOPSIS
+.BI "timeout [ \-s " signal " ] " timeout " " command " [ " args... " ]
+.SH DESCRIPTION
+The command
+.B timeout
+starts the command and will kill it after timeout seconds have passed.
+If the command times out, then we exit with status ETIMEDOUT,
+otherwise the normal exit status of the command is returned.
+If no signal is specified, the TERM signal is sent.  The TERM signal
+will kill processes which do not catch this signal.  For other processes,
+it may be necessary to use the KILL (9) signal, since this signal cannot
+be caught.
+.SH OPTIONS
+.TP
+.IR timeout ...
+Specify the timeout in seconds
+.TP
+.BI \-s " signal "
+Specify the signal number to send.
+.SH "SEE ALSO"
+.BR kill (1),
+.SH AUTHOR
+Padraig Brady <[EMAIL PROTECTED]>.
+.SH AVAILABILITY
+The timeout command is part of the util-linux-ng package and is available from
+ftp://ftp.kernel.org/pub/linux/utils/util-linux-ng/.
diff --git a/misc-utils/timeout.c b/misc-utils/timeout.c
new file mode 100644
index 000..f4bbd4b
--- /dev/null
+++ b/misc-utils/timeout.c
@@ -0,0 +1,188 @@
+/*
+ Copyright © 2008 Pádraig Brady <[EMAIL PROTECTED]>
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+*/
+
+/*
+ timeout - Start a command, and kill it if the specified timeout expires
+
+ We try to behave like a shell starting a single (foreground) job,
+ and will kill the job if we receive the alarm signal we setup.
+ If the jobs times out, then we exit with status ETIMEDOUT,
+ otherwise the normal exit status of the job is returned.
+*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int timed_out;
+static int term_signal=SIGTERM; /* same default as kill command */
+static int monitored_pid;
+static int sigs_to_ignore[NSIG]; /* so monitor can ignore sigs it resends */
+
+/* TODO: merge with kill.c */
+int arg_to_signum (char *arg)
+{
+int numsig;
+char *ep;
+
+if (isdigit(*arg)) {
+numsig = strtol(arg, &ep, 10);
+if (*ep!='\0' || numsig<0 || numsig>=NSIG)
+  

Re: Feature Request: du --dir-with-slash

2008-03-07 Thread Pádraig Brady
Thomas Guettler wrote:
> Hi,
> 
> I use "du -ax / | sort -rn > /var/tmp/du-`date --iso`" to get the sorted
> total size of all
> files and directories.
> 
> Unfortunately you can't see a difference between a directory and a file
> in the output. It would be nice if the directories would be added by a
> slash.

How about the attaced patch?

cheers,
Pádraig.
>From 0d9130e0bd8b4f09be181e8e7afabdacfb872482 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Fri, 7 Mar 2008 15:44:34 +
Subject: [PATCH] Ensure du outputs a trailing / for directories.
  * src/du.c: This allows one to distinguish directories
  in `du -a` output. Suggestion from Thomas Guettler <[EMAIL PROTECTED]>


Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 ChangeLog-2008 |5 +
 src/du.c   |   11 +++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ChangeLog-2008 b/ChangeLog-2008
index aac9feb..279530a 100644
--- a/ChangeLog-2008
+++ b/ChangeLog-2008
@@ -1,3 +1,8 @@
+2008-03-07  Pádraig Brady <[EMAIL PROTECTED]>
+
+	* src/du.c: Ensure the directories output by du
+	have a trailing '/' so that they can be distinguished in `du -a` output
+
 2008-02-07  Jim Meyering  <[EMAIL PROTECTED]>
 
 	We *do* need two different version files.
diff --git a/src/du.c b/src/du.c
index 6681079..25aaa99 100644
--- a/src/du.c
+++ b/src/du.c
@@ -437,7 +437,7 @@ print_only_size (uintmax_t n_bytes)
 /* Print size (and optionally time) indicated by *PDUI, followed by STRING.  */
 
 static void
-print_size (const struct duinfo *pdui, const char *string)
+print_size (const struct duinfo *pdui, const char *string, const struct stat *sb)
 {
   print_only_size (pdui->size);
   if (opt_time)
@@ -445,7 +445,10 @@ print_size (const struct duinfo *pdui, const char *string)
   putchar ('\t');
   show_date (time_format, pdui->tmax);
 }
-  printf ("\t%s%c", string, opt_nul_terminate_output ? '\0' : '\n');
+  printf ("\t%s%s%c",
+  string,
+  sb && S_ISDIR(sb->st_mode) ? "/" : "",
+  opt_nul_terminate_output ? '\0' : '\n');
   fflush (stdout);
 }
 
@@ -607,7 +610,7 @@ process_file (FTS *fts, FTSENT *ent)
 
   if ((IS_DIR_TYPE (ent->fts_info) && level <= max_depth)
   || ((opt_all && level <= max_depth) || level == 0))
-print_size (&dui_to_print, file);
+print_size (&dui_to_print, file, sb);
 
   return ok;
 }
@@ -653,7 +656,7 @@ du_files (char **files, int bit_flags)
 }
 
   if (print_grand_total)
-print_size (&tot_dui, _("total"));
+print_size (&tot_dui, _("total"), NULL);
 
   return ok;
 }
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature Request: du --dir-with-slash

2008-03-07 Thread Pádraig Brady
Eric Blake wrote:
> I don't think you can do it by default - it must be via an option.  POSIX
> states:
> 
> ~The output from du shall consist of the amount of space allocated to a
> file and the name of the file, in the following format:
> 
> ~"%d %s\n", , 
> 
> http://www.opengroup.org/onlinepubs/009695399/utilities/du.html
> (where ' ' implies an arbitrary amount of whitespace characters).
> 
> I don't think we can arbitrarily add / to  and still comply with
> POSIX.

So does that mean we should be stripping trailing /
from directories that are specified?

$ du -s /tmp
8040/tmp

I think adding a / to all dirs is therefore more consistent
as well as providing more info to the user.
I understand the need to comply with POSIX where possible,
but one needs to use common sense to move forward.
POSIX is not the be all and end all.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature Request: du --dir-with-slash

2008-03-10 Thread Pádraig Brady
Thomas Guettler wrote:
> Pádraig Brady schrieb:
>> Thomas Guettler wrote:
>>   
>>> Hi,
>>>
>>> I use "du -ax / | sort -rn > /var/tmp/du-`date --iso`" to get the sorted
>>> total size of all
>>> files and directories.
>>>
>>> Unfortunately you can't see a difference between a directory and a file
>>> in the output. It would be nice if the directories would be added by a
>>> slash.
>>> 
> Thank you for being interested. I wrote a patch which makes the slash
> optional:
> 
> src/du --help:
>  ...
>   -z, --dir-with-slash  append a slash to directories.


I don't think it's worth an option TBH.
How about we do this iff POSIXLY_CORRECT environment variable is not set?

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature Request: du --dir-with-slash

2008-03-10 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> Thomas Guettler wrote:
>>> Thank you for being interested. I wrote a patch which makes the slash
>>> optional:
>>>
>>> src/du --help:
>>>  ...
>>>   -z, --dir-with-slash  append a slash to directories.
>> I don't think it's worth an option TBH.
>> How about we do this iff POSIXLY_CORRECT environment variable is not set?
> 
> Hi Pádraig,
> 
> I don't like to encourage the use of POSIXLY_CORRECT,
> and fear such a change might end up doing that.

agree

> Besides, the cost[*]/benefit ratio of adding trailing slashes by
> default looks way too high.
> 
> [*] in potential portability hassles.
> I.e., people could easily come to rely on the GNU df's trailing slashes,
> and then inadvertently write non-portable scripts.

Ah, forwards compatibility. I hadn't considered that.
Therefore the correct path (pardon the pun) is
to get POSIX to update the standard if this is in fact
deemed an improvement warranting that, and work from there.

> I'm very dubious about adding this as an option.
> Do you really think an option is warranted for this,
> considering you can already get the desired behavior
> with a small wrapper, as I demonstrated:

The wrapper is awkward, but it's better than a new option.

So probably best to do nothing here as you suggest.

thanks for the clarifications,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature Request: du --dir-with-slash

2008-03-10 Thread Pádraig Brady
Thomas Guettler wrote:
>>> I'm very dubious about adding this as an option.
>>> Do you really think an option is warranted for this,
>>> considering you can already get the desired behavior
>>> with a small wrapper, as I demonstrated:
>>> 
>> The wrapper is awkward, but it's better than a new option.
>>
>> So probably best to do nothing here as you suggest.
>>
>>   
> 
> I know that it easy to use the 'du -a' and pipe the output to a
> check if it is a directory. But if you have a lot of files, it
> really matters if you touch a files twice.
> 
> And this solution does not count the size of directories.
> 
> find / -xdev -printf "%k\t%p%y\n" |
> sed 's/d$/\//;t; s/.$//;' |
> LC_ALL=C sort -rn -k1,1

Oops true. How about (I realize this is getting protracted):

find / -xdev -printf "%p%y\n" |
sed 's/d$/\//;t; s/.$//;' |
tr '\n' '\0' |
xargs -r0 du -s |
LC_ALL=C sort -rn -k1,1

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature Request: du --dir-with-slash

2008-03-10 Thread Pádraig Brady
Pádraig Brady wrote:
> find / -xdev -printf "%p%y\n" |
> sed 's/d$/\//;t; s/.$//;' |
> tr '\n' '\0' |
> xargs -r0 du -s |
> LC_ALL=C sort -rn -k1,1

Note that will give the output you want but will be very inefficient :(
So Jim's method is the best compromise for the moment.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: stat -f output standpoint

2008-03-16 Thread Pádraig Brady
Eric Blake wrote:
> According to [EMAIL PROTECTED] on 3/15/2008 10:33 PM:
> | (P.S., you might enjoy writing `' instead of "", but it just leads to
> | cut and paste accidents sent to the shell, for me at least, about
> | twice a year.
> 
> You do realize that it is possible to create a custom LC_MESSAGES locale
> which translates the strings "`" and "'" to your preferred quote strings?
> ~ For that matter, the [EMAIL PROTECTED] locale already does most of that - on
> capable terminals, it uses nicer-looking quotes and bolds the output (and
> since those quotes aren't active shell characters, copy-n-paste is less
> likely to cause accidents), and on less capable terminals, the output is
> transliterated to "\"" and "\"" rather than "`" and "'".

[EMAIL PROTECTED] doesn't change anything on my Fedora 8 or Ubuntu 7.10 installs
Anyway I don't think this should be locale dependent.

The main place I notice this is for the find man page.
If you cut & paste the examples from there you get
the funny quote characters. Given it's going to be
newer users that tend to do that, it will be especially confusing.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Problems on darwin with coreutils-6.10.133-677610

2008-03-26 Thread Pádraig Brady
Jim Meyering wrote:
> Eric Blake <[EMAIL PROTECTED]> wrote:
>> According to Jim Meyering on 3/25/2008 3:47 PM:
>> | -  user_name = pwd ? pwd->pw_name : NULL;
>> | +  user_name = pwd ? xstrdup (pwd->pw_name) : NULL;
>>
>> Doesn't this leak memory?
> 
> Thanks for mentioning it, but I think of this as a pseudo leak,
> since that allocated storage is still reachable at exit:
> 
> $ valgrind --leak-check=full --show-reachable=yes ./id -G
> ...
> ==18858== 9 bytes in 1 blocks are still reachable in loss record 1 of 4
> ==18858==at 0x4C20FAB: malloc (vg_replace_malloc.c:207)
> ==18858==by 0x403D49: xstrdup (in /f/w/cu/src/id)
> ==18858==by 0x40198E: main (id.c:214)
> ...
> 
> IMHO, it's best not even to try to free such memory,
> unless the total amount can become large enough to interfere
> with regular operation.

It's easier to use the tools though without this "noise".
For my own programs I often change main to run in an
infinite loop (with suitable ulimit) to check leaks.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Bug#467378: coreutils: Please include a program to truncate files

2008-03-26 Thread Pádraig Brady
Proposed truncate command attached
>From 34b9bc72ffe70ec83710b12021e889d5ae65e508 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Wed, 26 Mar 2008 14:03:31 +
Subject: [PATCH] Add new program: truncate
 * AUTHORS: Register as the author.
 * NEWS: Mention this change.
 * README: Add truncate command to list
 * src/truncate.c: New command.
 * src/Makefile.am: Add truncate command to list to build
 * doc/coreutils.texi (truncate invocation): Add truncate info.
 * man/Makefile.am: Add truncate man page to list to build
 * man/truncate.x: Add truncate man page template
 * tests/misc/Makefile.am: Add truncate tests
 * tests/misc/help-version: Add support for new truncate command.
 * tests/misc/truncate-dangling-symlink: check dangling link ok
 * tests/misc/truncate-dir-fail: ensure dirs fail
 * tests/misc/truncate-fail-diag: validate messages for missing paths
 * tests/misc/truncate-fifo: ensure fifos ignored
 * tests/misc/truncate-no-create-missing: ensure -c option honoured
 * tests/misc/truncate-overflow: check signed integer overflows
 * tests/misc/truncate-owned-by-other: root permissions check
 * tests/misc/truncate-parameters: check invalid parameter combinations
 * tests/misc/truncate-relative: check invalid relative sizes


Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 AUTHORS   |1 +
 ChangeLog-2008|   24 ++-
 NEWS  |4 +
 README|3 +-
 doc/coreutils.texi|   80 +++-
 man/Makefile.am   |1 +
 man/truncate.x|6 +
 src/Makefile.am   |2 +-
 src/truncate.c|  371 +
 tests/misc/Makefile.am|9 +
 tests/misc/help-version   |1 +
 tests/misc/truncate-dangling-symlink  |   35 +++
 tests/misc/truncate-dir-fail  |   14 ++
 tests/misc/truncate-fail-diag |   52 +
 tests/misc/truncate-fifo  |   38 
 tests/misc/truncate-no-create-missing |   31 +++
 tests/misc/truncate-overflow  |   52 +
 tests/misc/truncate-owned-by-other|   40 
 tests/misc/truncate-parameters|   43 
 tests/misc/truncate-relative  |   40 
 20 files changed, 841 insertions(+), 6 deletions(-)
 create mode 100644 man/truncate.x
 create mode 100644 src/truncate.c
 create mode 100755 tests/misc/truncate-dangling-symlink
 create mode 100755 tests/misc/truncate-dir-fail
 create mode 100755 tests/misc/truncate-fail-diag
 create mode 100755 tests/misc/truncate-fifo
 create mode 100755 tests/misc/truncate-no-create-missing
 create mode 100755 tests/misc/truncate-overflow
 create mode 100755 tests/misc/truncate-owned-by-other
 create mode 100755 tests/misc/truncate-parameters
 create mode 100755 tests/misc/truncate-relative

diff --git a/AUTHORS b/AUTHORS
index 807857f..8b33318 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -87,6 +87,7 @@ test: Kevin Braunsdorf, Matthew Bradburn
 touch: Paul Rubin, Arnold Robbins, Jim Kingdon, David MacKenzie, Randy Smith
 tr: Jim Meyering
 true: Jim Meyering
+truncate: Pádraig Brady
 tsort: Mark Kettenis
 tty: David MacKenzie
 uname: David MacKenzie
diff --git a/ChangeLog-2008 b/ChangeLog-2008
index 279530a..d692adb 100644
--- a/ChangeLog-2008
+++ b/ChangeLog-2008
@@ -0,0 +1,25 @@
+2008-03-26  Pádraig Brady <[EMAIL PROTECTED]>
 
+	Add new program: truncate
+	* AUTHORS: Register as the author.
+	* NEWS: Mention this change.
+	* README: Add truncate command to list
+	* src/truncate.c: New command.
+	* src/Makefile.am: Add truncate command to list to build
+	* doc/coreutils.texi (truncate invocation): Add truncate info.
+	* man/Makefile.am: Add truncate man page to list to build
+	* man/truncate.x: Add truncate man page template
+	* tests/misc/Makefile.am: Add truncate tests
+	* tests/misc/help-version: Add support for new truncate command.
+	* tests/misc/truncate-dangling-symlink: check dangling link ok
+	* tests/misc/truncate-dir-fail: ensure dirs fail
+	* tests/misc/truncate-fail-diag: validate messages for missing paths
+	* tests/misc/truncate-fifo: ensure fifos ignored
+	* tests/misc/truncate-no-create-missing: ensure -c option honoured
+	* tests/misc/truncate-overflow: check signed integer overflows
+	* tests/misc/truncate-owned-by-other: root permissions check
+	* tests/misc/truncate-parameters: check invalid parameter combinations
+	* tests/misc/truncate-relative: check invalid relative sizes
 
 2008-02-07  Jim Meyering  <[EMAIL PROTECTED]>
 
diff --git a/NEWS b/NEWS
index 948bced..e3d059d 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU coreutils NEWS-*- outline -*-
 
 * Noteworthy changes in release 6.?? (2008-??-??) [stable]
 
+** New programs
+
+  truncate: Set the size of a file to a specified size.
+
 ** Bug fixes
 
   configure --enable-no-install-

Re: Bug#467378: coreutils: Please include a program to truncate files

2008-03-26 Thread Pádraig Brady
Take 2 attached
>From 345b3af13f3e44d365535b93691ae897b5512805 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Thu, 27 Mar 2008 01:11:25 +
Subject: [PATCH] Add new program: truncate

* AUTHORS: Register as the author
* NEWS: Mention this change
* README: Add truncate command to list
* src/truncate.c: New command
* src/Makefile.am: Add truncate command to list to build
* doc/coreutils.texi (truncate invocation): Add truncate info
* man/Makefile.am: Add truncate man page to list to build
* man/truncate.x: Add truncate man page template
* po/POTFILES.in: Add truncate to list to translate
* tests/misc/Makefile.am: Add truncate tests
* tests/misc/help-version: Add support for new truncate command
* tests/misc/truncate-dangling-symlink: check dangling link ok
* tests/misc/truncate-dir-fail: ensure dirs fail
* tests/misc/truncate-fail-diag: validate messages for missing paths
* tests/misc/truncate-fifo: ensure fifos ignored
* tests/misc/truncate-no-create-missing: ensure -c option honoured
* tests/misc/truncate-overflow: check signed integer overflows
* tests/misc/truncate-owned-by-other: root permissions check
* tests/misc/truncate-parameters: check invalid parameter combinations
* tests/misc/truncate-relative: check invalid relative sizes

Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 AUTHORS   |1 +
 ChangeLog-2008|   24 ++
 NEWS  |4 +
 README|3 +-
 doc/coreutils.texi|   80 +++-
 man/Makefile.am   |1 +
 man/truncate.x|6 +
 po/POTFILES.in|1 +
 src/Makefile.am   |2 +-
 src/truncate.c|  382 +
 tests/Makefile.am |4 +-
 tests/misc/Makefile.am|9 +
 tests/misc/help-version   |1 +
 tests/misc/truncate-dangling-symlink  |   35 +++
 tests/misc/truncate-dir-fail  |   14 ++
 tests/misc/truncate-fail-diag |   52 +
 tests/misc/truncate-fifo  |   38 
 tests/misc/truncate-no-create-missing |   31 +++
 tests/misc/truncate-overflow  |   52 +
 tests/misc/truncate-owned-by-other|   40 
 tests/misc/truncate-parameters|   43 
 tests/misc/truncate-relative  |   40 
 22 files changed, 859 insertions(+), 4 deletions(-)
 create mode 100644 man/truncate.x
 create mode 100644 src/truncate.c
 create mode 100755 tests/misc/truncate-dangling-symlink
 create mode 100755 tests/misc/truncate-dir-fail
 create mode 100755 tests/misc/truncate-fail-diag
 create mode 100755 tests/misc/truncate-fifo
 create mode 100755 tests/misc/truncate-no-create-missing
 create mode 100755 tests/misc/truncate-overflow
 create mode 100755 tests/misc/truncate-owned-by-other
 create mode 100755 tests/misc/truncate-parameters
 create mode 100755 tests/misc/truncate-relative

diff --git a/AUTHORS b/AUTHORS
index 807857f..89e82b7 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -87,6 +87,7 @@ test: Kevin Braunsdorf, Matthew Bradburn
 touch: Paul Rubin, Arnold Robbins, Jim Kingdon, David MacKenzie, Randy Smith
 tr: Jim Meyering
 true: Jim Meyering
+truncate: Padraig Brady
 tsort: Mark Kettenis
 tty: David MacKenzie
 uname: David MacKenzie
diff --git a/ChangeLog-2008 b/ChangeLog-2008
index aac9feb..5de93c6 100644
--- a/ChangeLog-2008
+++ b/ChangeLog-2008
@@ -1,3 +1,27 @@
+2008-03-26  Pádraig Brady <[EMAIL PROTECTED]>
+
+	Add new program: truncate
+	* AUTHORS: Register as the author
+	* NEWS: Mention this change
+	* README: Add truncate command to list
+	* src/truncate.c: New command
+	* src/Makefile.am: Add truncate command to list to build
+	* doc/coreutils.texi (truncate invocation): Add truncate info
+	* man/Makefile.am: Add truncate man page to list to build
+	* man/truncate.x: Add truncate man page template
+	* po/POTFILES.in: Add truncate to list to translate
+	* tests/misc/Makefile.am: Add truncate tests
+	* tests/misc/help-version: Add support for new truncate command
+	* tests/misc/truncate-dangling-symlink: check dangling link ok
+	* tests/misc/truncate-dir-fail: ensure dirs fail
+	* tests/misc/truncate-fail-diag: validate messages for missing paths
+	* tests/misc/truncate-fifo: ensure fifos ignored
+	* tests/misc/truncate-no-create-missing: ensure -c option honoured
+	* tests/misc/truncate-overflow: check signed integer overflows
+	* tests/misc/truncate-owned-by-other: root permissions check
+	* tests/misc/truncate-parameters: check invalid parameter combinations
+	* tests/misc/truncate-relative: check invalid relative sizes
+
 2008-02-07  Jim Meyering  <[EMAIL PROTECTED]>
 
 	We *do* need two different version files.
diff --git a/NEWS b/NEWS
index 5250ed8..42b9774 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU coreutils NEWS-*- outlin

[PATCH] Add timeout utility

2008-04-01 Thread Pádraig Brady
attached
>From e2c2d212bfea29540bb433bb8d00e1e9e9fd3ff6 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Fri, 28 Mar 2008 11:05:55 +
Subject: [PATCH] Add new program: timeout

* AUTHORS: Register as the author
* NEWS: Mention this change
* README: Add timeout command to list
* src/truncate.c: New command
* src/Makefile.am: Add timeout command to list to build
* doc/coreutils.texi (timeout invocation): Add timeout info
* man/Makefile.am: Add timeout man page to list to build
* man/truncate.x: Add timeout man page template
* po/POTFILES.in: Add timeout to list to translate
* tests/misc/Makefile.am: Add timeout tests
* tests/misc/help-version: Add support for new timeout command
* tests/misc/timeout: check basic timeout operation
* tests/misc/timeout-parameters: check invalid parameter combinations

Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 AUTHORS   |1 +
 NEWS  |4 +
 README|4 +-
 doc/coreutils.texi|  242 +--
 man/Makefile.am   |1 +
 man/timeout.x |6 +
 po/POTFILES.in|1 +
 src/Makefile.am   |2 +-
 src/timeout.c |  370 +
 tests/misc/Makefile.am|2 +
 tests/misc/help-version   |2 +
 tests/misc/timeout|   41 +
 tests/misc/timeout-parameters |   58 +++
 13 files changed, 646 insertions(+), 88 deletions(-)
 create mode 100644 man/timeout.x
 create mode 100644 src/timeout.c
 create mode 100755 tests/misc/timeout
 create mode 100755 tests/misc/timeout-parameters

diff --git a/AUTHORS b/AUTHORS
index 807857f..87553fd 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -84,6 +84,7 @@ tac: Jay Lepreau, David MacKenzie
 tail: Paul Rubin, David MacKenzie, Ian Lance Taylor, Jim Meyering
 tee: Mike Parker, Richard M. Stallman, David MacKenzie
 test: Kevin Braunsdorf, Matthew Bradburn
+timeout: Padraig Brady
 touch: Paul Rubin, Arnold Robbins, Jim Kingdon, David MacKenzie, Randy Smith
 tr: Jim Meyering
 true: Jim Meyering
diff --git a/NEWS b/NEWS
index 5250ed8..a326991 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU coreutils NEWS-*- outline -*-
 
 * Noteworthy changes in release 6.?? (2008-??-??) [stable]
 
+** New programs
+
+  timeout: Run a command with bounded time.
+
 ** Bug fixes
 
   configure --enable-no-install-program=groups now works.
diff --git a/README b/README
index 7a608f4..6159dfd 100644
--- a/README
+++ b/README
@@ -13,8 +13,8 @@ The programs that can be built with this package are:
   link ln logname ls md5sum mkdir mkfifo mknod mktemp mv nice nl nohup
   od paste pathchk pinky pr printenv printf ptx pwd readlink rm rmdir
   runcon seq sha1sum sha224sum sha256sum sha384sum sha512sum shred shuf
-  sleep sort split stat stty su sum sync tac tail tee test touch tr true
-  tsort tty uname unexpand uniq unlink uptime users vdir wc who whoami yes
+  sleep sort split stat stty su sum sync tac tail tee test timeout touch tr
+  true tsort tty uname unexpand uniq unlink uptime users vdir wc who whoami yes
 
 See the file NEWS for a list of major changes in the current release.
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index f161c4d..61d0af1 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -111,6 +111,7 @@
 * tail: (coreutils)tail invocation. Output the last part of files.
 * tee: (coreutils)tee invocation.   Redirect to multiple files.
 * test: (coreutils)test invocation. File/string tests.
+* timeout: (coreutils)touch invocation. Run with time limit.
 * touch: (coreutils)touch invocation.   Change file timestamps.
 * tr: (coreutils)tr invocation. Translate characters.
 * true: (coreutils)true invocation. Do nothing, successfully.
@@ -190,7 +191,7 @@ Free Documentation License''.
 * Working context::pwd stty printenv tty
 * User information::   id logname whoami groups users who
 * System context:: date uname hostname hostid
-* Modified command invocation::chroot env nice nohup su
+* Modified command invocation::chroot env nice nohup su timeout
 * Process control::kill
 * Delaying::   sleep
 * Numeric operations:: factor seq
@@ -208,6 +209,7 @@ Common Options
 * Exit status:: Indicating program success or failure.
 * Backup options::  Backup options
 * Block size::  Block size
+* Signal specifications::   Specifying signals
 * Disambiguating names and IDs:: chgrp and chown owner and group syntax
 * Random sources::  Sources of random data
 * Target directory::Target directory
@@ -421,6 +423,7 @@ Modified command invocation
 * nice invocation::

Re: [PATCH] Add timeout utility

2008-04-02 Thread Pádraig Brady
Mike Frysinger wrote:
> On Tuesday 01 April 2008, Pádraig Brady wrote:
>> +  /* Setup handlers before fork() so that we
>> +   * handle any signals caused by child, without races.  */
>> +  signal (SIGALRM, cleanup);/* our timeout.  */
>> +  signal (SIGINT, cleanup); /* Ctrl-C at terminal for example.  */
>> +  signal (SIGQUIT, cleanup);/* Ctrl-\ at terminal for example.  */
>> +  signal (SIGTERM, cleanup);/* if we're killed, stop monitored
>> process.  */ +  signal (SIGHUP, cleanup); /* terminal closed for
>> example.  */ +  signal (SIGTTIN, SIG_IGN);/* don't sTop if background
>> child needs tty.  */ +  signal (SIGTTOU, SIG_IGN);/* don't sTop if
>> background child needs tty.  */
> 
> if you're using signal(), you have race problems.  why not use sigaction() 
> and 
> friends instead ?

I originally wrote this for util-linux-ng.
BSD and linux at least handle signal() sanely
so I wrote it for those.

Since coreutils needs to be more portable,
I'll change to the more complicated sigaction instead.

To be sure, are you referring to races where a signal
can be received while in the signal handler on some systems?

Also there is the issue of restarting system calls
after the signal handler has run.

Also there is the issue of resetting the signal
handler after it has run.

Are there systems that coreutils supports currently
that doesn't follow BSD/linux semantics wrt the above 3 points?

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Add timeout utility

2008-04-02 Thread Pádraig Brady
Jim Meyering wrote:
> Thanks!
> 
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> Subject: [PATCH] Add new program: timeout
> ...
>> +/* Given an integer value *X, and a suffix character, SUFFIX_CHAR,
>> +   scale *X by the multiplier implied by SUFFIX_CHAR.  SUFFIX_CHAR may
>> +   be the NUL byte or `s' to denote seconds, `m' for minutes, `h' for
>> +   hours, or `d' for days.  If SUFFIX_CHAR is invalid, don't modify *X
>> +   and return false.  If *X would overflow, don't modify *X and return 
>> false.
>> +   Otherwise return true.  */
>> +
>> +static bool
>> +apply_suffix (unsigned int *x, char suffix_char)
> 
> You probably expect this, but I have to say it ;-)
> Don't duplicate code!

of course.

> apply_suffix is a tiny function, and used also by sleep.c, so let's not
> duplicate it.  How about putting it (static inline) in system.h instead?

sleep's apply_suffic() works on floats where as timeout's work on ints.
I'll see if I can merge them somewhat.
Hmm I could allow float input to timeout (1.5d for example),
and then round this to the nearest second later?

> operand2sig is similar but not as lightweight, since it uses str2sig,
> error, xstrdup and W* macros.  Maybe put it in its own file in src/?
> Or change xstrdup to strdup, eliminate the error call, and consider
> putting it in lib/gnulib.  It's probably better to go the easier
> route and use a separate file in src/[ch].

I intended but forgot to add the comment:
/* FIXME: where will we refactor this to? */
I'll add a new src/ file so.

> 
> Rather than '???' (or in addition to), please mark such
> comments with "FIXME".

righto

>> +  /* Setup handlers before fork() so that we
>> +   * handle any signals caused by child, without races.  */
>> +  signal (SIGALRM, cleanup);/* our timeout.  */
>> +  signal (SIGINT, cleanup); /* Ctrl-C at terminal for example.  */
>> +  signal (SIGQUIT, cleanup);/* Ctrl-\ at terminal for example.  */
>> +  signal (SIGTERM, cleanup);/* if we're killed, stop monitored process. 
>>  */
> 
> Mike Frysinger's point is a good one: use signal only if absolutely
> necessary.  Prefer sigaction.
> 
> Most signal-handling code in coreutils uses sigaction, and falls back on
> signal ifndef SA_NOCLDSTOP.  But the ifdefs and code duplication are
> pretty ugly.  If you're game, I'd like to try using *only* sigaction for
> an initial test release, in order to see if any "reasonable portability
> targets" remain that lack sigaction support.  Then, if there are no
> build failure reports for a long enough period, we can remove the crufty
> signal-using #else blocks in a bunch of programs.


Ok, I'll assume that sigaction is available.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Add timeout utility

2008-04-02 Thread Pádraig Brady
Bo Borgerson wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> Subject: [PATCH] Add new program: timeout
> 
> Great idea for a tool!

Not my idea TBH:
http://mail.linux.ie/pipermail/ilug/2006-November/thread.html#90654

> Have you considered an alternate run-mode where it could operate as a
> filter and timeout on 'inactivity' of the pipeline?
> 
> For example:
> 
> $ sort -m inputs/* | timeout --inactivity 1m program_prone_to_stalling
> 
> Where timeout would open a pipe and dup2 the read end to child's
> STDIN_FILENO before exec'ing.

How would `timeout` determine when
`program_prone_to_stalling` issues read()s ?

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Add timeout utility

2008-04-02 Thread Pádraig Brady
Mike Frysinger wrote:
> On Wednesday 02 April 2008, Pádraig Brady wrote:
>> To be sure, are you referring to races where a signal
>> can be received while in the signal handler on some systems?
>>
>> Also there is the issue of restarting system calls
>> after the signal handler has run.
>>
>> Also there is the issue of resetting the signal
>> handler after it has run.
>>
>> Are there systems that coreutils supports currently
>> that doesn't follow BSD/linux semantics wrt the above 3 points?
> 
> afaik, Linux behaves as POSIX dictates.  that means it does not behave as you 
> describe above.  newer versions of glibc may block the same signal from 
> re-occuring, but it doesnt block other signals.

True, but the current timeout implementation can handle that.
Anyway I'll get rid of the ambiguity using sigaction as you suggest.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Add timeout utility

2008-04-02 Thread Pádraig Brady
Bo Borgerson wrote:
> On Wed, Apr 2, 2008 at 10:20 AM, Pádraig Brady <[EMAIL PROTECTED]> wrote:
>>  It will always go through though as the kernel will buffer it.
> 
> Yes, that introduces some fuzz, but I think the principle remains
> viable -- the kernel will only buffer so much.

That could be a long time though.

> 
> Consider the following using a timeout.c modified with the attached
> patch, and a small Perl program (below) than hangs after 10 seconds:
> 
> $ time yes | src/timeout -i 2s ./write_then_hang 10 >/dev/null

replacing `yes` with `while true; do yes; sleep 1; done`
causes this to hang for ages.

I'll have another look at this when I finalize `timeout`

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Add timeout utility

2008-04-03 Thread Pádraig Brady
take 2 attached.
Note I kept the time interval parsing function
separate between `kill` and `timeout`, as they
operate on different types.

Pádraig.
>From 95c61d20933a749482c3d1d54eb3b1032735ab75 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Fri, 28 Mar 2008 11:05:55 +
Subject: [PATCH] Add new program: timeout

* AUTHORS: Register as the author
* NEWS: Mention this change
* README: Add timeout command to list
* src/truncate.c: New command
* src/operand2sig.c: Split out function called by kill and timeout
* src/operand2sig.h: New operand2sig() function prototype
* src/Makefile.am: Add timeout command to list to build
* doc/coreutils.texi (timeout invocation): Add timeout info
* man/Makefile.am: Add timeout man page to list to build
* man/truncate.x: Add timeout man page template
* po/POTFILES.in: Add timeout to list to translate
* tests/misc/Makefile.am: Add timeout tests
* tests/misc/help-version: Add support for new timeout command
* tests/misc/timeout: check basic timeout operation
* tests/misc/timeout-parameters: check invalid parameter combinations

Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 AUTHORS   |1 +
 NEWS  |4 +
 README|4 +-
 doc/coreutils.texi|  242 +++---
 man/Makefile.am   |1 +
 man/timeout.x |6 +
 po/POTFILES.in|1 +
 src/Makefile.am   |5 +-
 src/kill.c|   47 +--
 src/operand2sig.c |   84 ++
 src/operand2sig.h |   19 +++
 src/timeout.c |  335 +
 tests/misc/Makefile.am|2 +
 tests/misc/help-version   |2 +
 tests/misc/timeout|   44 ++
 tests/misc/timeout-parameters |   55 +++
 16 files changed, 718 insertions(+), 134 deletions(-)
 create mode 100644 man/timeout.x
 create mode 100644 src/operand2sig.c
 create mode 100644 src/operand2sig.h
 create mode 100644 src/timeout.c
 create mode 100755 tests/misc/timeout
 create mode 100755 tests/misc/timeout-parameters

diff --git a/AUTHORS b/AUTHORS
index 807857f..87553fd 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -84,6 +84,7 @@ tac: Jay Lepreau, David MacKenzie
 tail: Paul Rubin, David MacKenzie, Ian Lance Taylor, Jim Meyering
 tee: Mike Parker, Richard M. Stallman, David MacKenzie
 test: Kevin Braunsdorf, Matthew Bradburn
+timeout: Padraig Brady
 touch: Paul Rubin, Arnold Robbins, Jim Kingdon, David MacKenzie, Randy Smith
 tr: Jim Meyering
 true: Jim Meyering
diff --git a/NEWS b/NEWS
index 5250ed8..a326991 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU coreutils NEWS-*- outline -*-
 
 * Noteworthy changes in release 6.?? (2008-??-??) [stable]
 
+** New programs
+
+  timeout: Run a command with bounded time.
+
 ** Bug fixes
 
   configure --enable-no-install-program=groups now works.
diff --git a/README b/README
index 7a608f4..6159dfd 100644
--- a/README
+++ b/README
@@ -13,8 +13,8 @@ The programs that can be built with this package are:
   link ln logname ls md5sum mkdir mkfifo mknod mktemp mv nice nl nohup
   od paste pathchk pinky pr printenv printf ptx pwd readlink rm rmdir
   runcon seq sha1sum sha224sum sha256sum sha384sum sha512sum shred shuf
-  sleep sort split stat stty su sum sync tac tail tee test touch tr true
-  tsort tty uname unexpand uniq unlink uptime users vdir wc who whoami yes
+  sleep sort split stat stty su sum sync tac tail tee test timeout touch tr
+  true tsort tty uname unexpand uniq unlink uptime users vdir wc who whoami yes
 
 See the file NEWS for a list of major changes in the current release.
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index f161c4d..61d0af1 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -111,6 +111,7 @@
 * tail: (coreutils)tail invocation. Output the last part of files.
 * tee: (coreutils)tee invocation.   Redirect to multiple files.
 * test: (coreutils)test invocation. File/string tests.
+* timeout: (coreutils)touch invocation. Run with time limit.
 * touch: (coreutils)touch invocation.   Change file timestamps.
 * tr: (coreutils)tr invocation. Translate characters.
 * true: (coreutils)true invocation. Do nothing, successfully.
@@ -190,7 +191,7 @@ Free Documentation License''.
 * Working context::pwd stty printenv tty
 * User information::   id logname whoami groups users who
 * System context:: date uname hostname hostid
-* Modified command invocation::chroot env nice nohup su
+* Modified command invocation::chroot env nice nohup su timeout
 * Process control::kill
 * Delaying::   sleep
 * Numeric operations:: factor seq
@@ -208,6 +209,7 @@ Common 

Re: miscutils

2008-04-06 Thread Pádraig Brady
James Youngman wrote:
> On Wed, Nov 15, 2006 at 1:19 AM, Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> It has been proposed to start the miscutils project,
>>  to include utilities not seen as core to the system.
>>  I.E. utilities that are of less general use, but
>>  still worth maintaining and distributing.
> 
> Did this ever get started?   I tried searching on Savannah and on the
> web, but without luck.There seem to be many hits, but none seemed
> relevant.
> 
> I was reminded of this by a remark that Jim made about it the other day.

No it was not officially started as I asked around for
what people would like included but got very few suggestions.
There were a couple of utils that I had (truncate & timeout),
but they are now queued up for inclusion in coreutils.

So I'm not sure another project is required/desired.
There is the moreutils project already in debian/ubuntu:
http://kitenet.net/~joey/code/moreutils.html
So I would suggest people direct to that and
I offer to help with that project.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Add new program: magic

2008-04-10 Thread Pádraig Brady
Matthew Woehlke wrote:
> [EMAIL PROTECTED] wrote:
>> | So far, it includes the following utilities:
>> |  - sponge: soak up standard input and write to a file
> 
> Eh? That sounds like 'cat > file'...

I was wondering that too.
It basically buffers the file in mem
so that one can modify a file through a filter
without the need for explicit temporary files.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


dd skip bug?

2008-04-16 Thread Pádraig Brady
dd handles skip weirdly

disk=/dev/sda8
dd if=$disk bs=8M count=1 skip=1000 of=/dev/null  #ok
dd if=$disk bs=8M count=1 skip=1000K of=/dev/null #reads whole disk! as seek 
fails

I had a 10s look at the source and noticed a comment
saying POSIX doesn't specify what we should do when
skipping past the end of input. For seekable files though,
reading the whole thing is unexpected to me at least.
I would expect it to do:

if (seekable && !seek(skip_len))
exit(EXIT_FAILURE);

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: dd skip bug?

2008-04-17 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> dd handles skip weirdly
>>
>> disk=/dev/sda8
>> dd if=$disk bs=8M count=1 skip=1000 of=/dev/null  #ok
>> dd if=$disk bs=8M count=1 skip=1000K of=/dev/null #reads whole disk! as seek 
>> fails
>>
>> I had a 10s look at the source and noticed a comment
>> saying POSIX doesn't specify what we should do when
>> skipping past the end of input. For seekable files though,
>> reading the whole thing is unexpected to me at least.
>> I would expect it to do:
>>
>> if (seekable && !seek(skip_len))
>> exit(EXIT_FAILURE);
> 
> Thanks, but the existing behavior is deliberate, and IMHO, necessary.
> 
> skip=N is required to try to seek, and failing that, position
> the read pointer by calling read.  That is so it works on
> e.g., redirected stdin as well as on regular files.

redirected stdin is seekable.
Note the logic I presented above.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: dd skip bug?

2008-04-18 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
> 
>> Jim Meyering wrote:
>>> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>>>> dd handles skip weirdly
>>>>
>>>> disk=/dev/sda8
>>>> dd if=$disk bs=8M count=1 skip=1000 of=/dev/null  #ok
>>>> dd if=$disk bs=8M count=1 skip=1000K of=/dev/null #reads whole disk! as 
>>>> seek fails
>>>>
>>>> I had a 10s look at the source and noticed a comment
>>>> saying POSIX doesn't specify what we should do when
>>>> skipping past the end of input. For seekable files though,
>>>> reading the whole thing is unexpected to me at least.
>>>> I would expect it to do:
>>>>
>>>> if (seekable && !seek(skip_len))
>>>> exit(EXIT_FAILURE);
>>> Thanks, but the existing behavior is deliberate, and IMHO, necessary.
>>>
>>> skip=N is required to try to seek, and failing that, position
>>> the read pointer by calling read.  That is so it works on
>>> e.g., redirected stdin as well as on regular files.
>> redirected stdin is seekable.
>> Note the logic I presented above.
> 
> Hi Pádraig,
> 
> Redirected stdin is seekable, as long as it's from a seekable file.
> I meant "piped stdin".  Same holds for any other non-seekable input source.
> I think if you try the code above, it will cause test failures.
> 

There are actually 3 cases to consider with large skip values.
Currently dd does the following for various values of skip:

if skip > file_size && skip < max_file_size
  lseek returns new offset, and read() returns 0

if skip > file_size && skip > max file size
  lseek returns error and read() used to advance input

if skip would overflow off_t
  read() used to advance input

It should at least be consistent for all cases I think.
I.E. for cases 2 & 3, we should seek to the end of the file
so that read() will just return 0 as in case 1.

A patch to do the above is attached.
Note I haven't tested this fully, it's just for illustration.

The more contentious thing to do in all 3 cases is to terminate with an error,
or just print a warning would be a good first step I suppose.
Exiting though would allow one for example to terminate a script that reads a 
file
in chunks with dd. The only way I can see to do this at present, is to use the
file size and chunk number ourselves to terminate the loop in the script.

thanks,
Pádraig
diff --git a/src/dd.c b/src/dd.c
index 0a7b154..76739f9 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1178,6 +1178,7 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
   char *buf)
 {
   uintmax_t offset = records * blocksize;
+  off_t soffset;
 
   /* Try lseek and if an error indicates it was an inappropriate operation --
  or if the file offset is not representable as an off_t --
@@ -1194,6 +1195,11 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
   else
 {
   int lseek_errno = errno;
+  if (fdesc == STDIN_FILENO && input_seekable)
+	{
+	  soffset = lseek(fdesc, 0, SEEK_END);
+	  advance_input_offset (soffset);
+	}
 
   do
 	{
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: dd skip bug?

2008-04-18 Thread Pádraig Brady
Pádraig Brady wrote:
> There are actually 3 cases to consider with large skip values.
> Currently dd does the following for various values of skip:
> 
> if skip > file_size && skip < max_file_size
>   lseek returns new offset, and read() returns 0
> 
> if skip > file_size && skip > max file size
>   lseek returns error and read() used to advance input
> 
> if skip would overflow off_t
>   read() used to advance input

4 cases actually:

 if skip > device_size
   read() used to advance input

Hopefully the attached patch makes dd more efficient/correct
in these cases. It passes the tests at least.

cheers,
Pádraig.
>From ccac25668b04d1c62f93dcec5419adb564db3ddd Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Fri, 18 Apr 2008 21:15:59 +0100
Subject: [PATCH] Stop dd doing redundant reading with erroneous skip offsets

* src/dd.c: Try to seek to the end of the file to stop
redundant reads when for example skip > max_file_size,
offset > dev_size, offset > OFF_T_MAX

Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 src/dd.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/dd.c b/src/dd.c
index 0a7b154..711bab7 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1178,6 +1178,7 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
   char *buf)
 {
   uintmax_t offset = records * blocksize;
+  off_t soffset;
 
   /* Try lseek and if an error indicates it was an inappropriate operation --
  or if the file offset is not representable as an off_t --
@@ -1194,6 +1195,17 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
   else
 {
   int lseek_errno = errno;
+  if (fdesc == STDIN_FILENO && input_seekable)
+	{
+	  /* We probably were asked to skip past the end of device.
+	   * Therefore try to seek to the end of the file,
+	   * so that subsequent reads will return immediately.  */
+	  if ((soffset = lseek (fdesc, 0, SEEK_END)) >= 0)
+	{
+	  advance_input_offset (soffset);
+	  return 0;
+	}
+	}
 
   do
 	{
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: coreutils and i18n

2008-04-21 Thread Pádraig Brady
Bruno Haible wrote:
> Jim Meyering wrote:
>>>   - Processing in unibyte locales should not become significantly slower
>>> than before.
>>>   - Code duplication should be avoided, for maintainability.
>>>   - Macros which expand to one thing in the multibyte case and to another
>>> thing for the unibyte case are not acceptable.
>>>
>>> How will this students' project solve this dilemma?
>> There's no guarantee, but Paul and I will be supervising.
> 
> I mean, what is technically the solution to the dilemma? The typical idiom
> for keeping the speed of the unibyte case is - see e.g. 
> gnulib/lib/mbscasecmp.c
> as an example -
> 
>   #if HAVE_MBRTOWC
> if (MB_CUR_MAX > 1)
>   ... multibyte case ...
> else
>   #endif
>   ... unibyte case ...
> 
> but it does have code duplication.

That's the obvious solution that is not really required/desired.

If I was being paid to do it (I have very little free time unfortunately),
then I would do something like...

1. identify filters that require multibyte handling.
2. refactor line input processing etc. to shared code.
3. Intelligently apply multibyte processing.

For illustration look at the performance various `uniq` implementations 
currently:

$ rpm -q coreutils
coreutils-6.9-9.fc8

$ echo $LANG
en_IE.UTF-8

# The default one uses the existing i18n patch
$ time uniq < lines.test > /dev/null
real0m27.724s

$ time LC_CTYPE=C uniq < lines.test > /dev/null
real0m1.314s

$time ~/git/coreutils/src/uniq < lines.test > /dev/null
real0m1.187s

$ time ~/myuniq < lines.test > /dev/null
real0m0.827s

$ time ~/uniq.py < lines.test > /dev/null
real0m2.657s

Yes the python version (which I nearly wrote in the same
time and the default uniq took to complete the test) is much better!

`myuniq` is a version I implemented from scratch,
to understand some of what the issues involved would be:
http://lists.gnu.org/archive/html/bug-coreutils/2006-07/msg00153.html

It's not just performance. The functionality of the i18n patch for uniq
is buggy in the presence of NUL characters for example:

for i in 1 2 3; do echo -e "1234\x0056789"; done | uniq
123456789
123456789
123456789

for i in 1 2 3; do echo -e "1234\x0056789"; done | LANG=C uniq
123456789

It's great that Paul & Jim are looking at this interesting project
as it really is important as I've mentioned before.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature request - base64 Filename Safe Alphabet

2008-04-29 Thread Pádraig Brady
Christopher Kerr wrote:
> After being burned by using `head -c6 /dev/urandom | base64` as part of a 
> directory name, I realised that it would be useful if base64 had an option to 
> generate URL and Filename safe encodings, as specified in RFC 3548 section 4.
> 
> This would make
> cat FILE | base64 --filename-safe
> equivalent to
> cat FILE | base64 | tr '+/' '-_'

Not a bad idea. I've needed stuff like that before:
http://www.pixelbeat.org/libs/base64.c

Perhaps `tr '+/' '._'` would be better so that
you don't need to worry about "-" at the start of a filename?

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Feature request - base64 Filename Safe Alphabet

2008-04-29 Thread Pádraig Brady
Bo Borgerson wrote:
> Pádraig Brady wrote:
>> Perhaps `tr '+/' '._'` would be better so that
>> you don't need to worry about "-" at the start of a filename?
> 
> 
> I'm think `.' at the beginning of a filename also has the potential to
> give users unexpected behavior.

Doh! never thought of that :)

tr '+/' '._' => hidden files
tr '+/' '-_' => awkward option clashes
tr '/' '_' => not POSIX portable

ho hum, the awkward option clashes is probably best.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: horrible utf-8 performace in wc

2008-05-07 Thread Pádraig Brady
Jan Engelhardt wrote:
> 
> https://bugzilla.novell.com/show_bug.cgi?id=381873
> 
> Forwarding this because it is a GNU issue, not specifically a Novell one.
> I reproduced this myself with the latest coreutils from git
> (BTW: You might want to repack that repo, "counting objects" during the
> clone was rather slow in the initial counting.)
> 
> Could it be a libiconv problem?

Accounting for multibyte characters is what's taking the time:

~/git/coreutils/src$ time ./wc -m long_lines.txt
13357046 long_lines.txt
real0m1.860s

~/git/coreutils/src$ time ./wc -c long_lines.txt
13538735 long_lines.txt
real0m0.002s

Now that is a _lot_ of extra time. libiconv could probably be
made more efficient. I've never actually looked at it.
However wc calls mbrtowc() for each multibyte character.
It would probably be a lot more efficient to use mbstowcs()
to convert the whole read buffer.

Note mbstowcs doesn't handle embedded NULs so one would
need to find these first, and iterate over each substring,
as I did in my version of uniq previously mentioned.

Also mbstowcs doesn't canonicalize equivalent multibyte sequences,
and so therefore functions the same in this regard as our
processing of each wide character separately.
This could be considered a bug actually- i.e. should -m give
the number of wide chars, or the number of multibyte chars?
With the attached patch, `wc -m` gives 23 chars for both these lines.

canonically équivalent
canonically équivalent

Pádraig.

p.s. I Notice that gnome-terminal still doesn't handle
combining characters correctly, and my mail client thunderbird
is putting the accent on the q rather than the e, sigh.
diff --git a/src/wc.c b/src/wc.c
index 61ab485..f7f7109 100644
--- a/src/wc.c
+++ b/src/wc.c
@@ -368,6 +368,8 @@ wc (int fd, char const *file_x, struct fstatus *fstatus)
 			linepos += width;
 			  if (iswspace (wide_char))
 			goto mb_word_separator;
+			  else if (width == 0)
+			chars--; /* don't count combining chars */
 			  in_word = true;
 			}
 		  break;
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: horrible utf-8 performace in wc

2008-05-07 Thread Pádraig Brady
Bo Borgerson wrote:
> Pádraig Brady wrote:
>> canonically équivalent
>> canonically équivalent
>>
>> Pádraig.
>>
>> p.s. I Notice that gnome-terminal still doesn't handle
>> combining characters correctly, and my mail client thunderbird
>> is putting the accent on the q rather than the e, sigh.
> 
> They both render correctly here (Thunderbird 2.0.0.12).

Ha, the viewer is OK actually but the composer combines
with the succeeding character (thunderbird 2.0.0.6).

> Is there a good library for combining-character canonicalization
> available?  That seems like something that would be useful to have in a
> lot of text-processing tools.  Also, for Unicode, something to shuffle
> between the normalization forms might be helpful for comparisons.

Yes that will be needed for when we get coreutils to support multibyte fully.
ICU has support for this, and Jim Meyering mentioned that there may
be support for this already in gnulib, but I haven't had a chance
to check it out yet.

> I may be misinterpreting your patch, but it seems to me that
> decrementing count for zero-width characters could potentially lead to
> confusion.  Not all zero-width characters are combining characters, right?

Yes you're probably right. I must write a little prog to check for exceptions.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: horrible utf-8 performace in wc

2008-05-07 Thread Pádraig Brady
Bo Borgerson wrote:
> Jim Meyering wrote:
>> Bo Borgerson <[EMAIL PROTECTED]> wrote:
>>> I may be misinterpreting your patch, but it seems to me that
>>> decrementing count for zero-width characters could potentially lead to
>>> confusion.  Not all zero-width characters are combining characters, right?
>> It looks ok to me, since there's an unconditional increment
>>
>>chars++;
>>
>> about 25 lines above, so the decrement would just undo that.
> 
> 
> Right, I guess my question is more about the semantics of `wc -m'.
> Should stand-alone zero-width characters such as the zero-width space be
> counted?
> 
> The attached (UTF-8) file contains 3 characters according to HEAD, but
> only two with the patch.

Interesting, I thought of that myself
but assumed iswspace(u"zero-width space") == 1
Actually there are no chars where:
  wcwidth(char)==0 && iswspace(char)==1

In the first 65535 code points there are also 404 chars which are
not classed as combining in the unicode database, but are classed
as zero width in the glibc locale data at least (zero-width space
being one of them like you mentioned). I determined this with the
attached progs:

./zw | python unidata.py | grep " 0 " | wc -l

So I suggest that we don't merge my tweak as is. What we could do is:
1. Find a method to distinguish the above 404 characters at least.
2. Define -m to mean "individual displayable characters" if this is
   what people usually want.
3. Add a new option for this.

Pádraig.
#define _GNU_SOURCE
#include 
#include 
#include 
#include 
#include 
#include 
#include 

int main(int argc, char** argv) {

/* This is a single threaded app, so mark as such for performance. */
#include 
__fsetlocking(stdin,FSETLOCKING_BYCALLER);
__fsetlocking(stdout,FSETLOCKING_BYCALLER);

if (!setlocale(LC_CTYPE, "")) { //TODO: What about LC_COLLATE?
   fprintf(stderr,"Warning locale not supported by glibc, using 'C' locale\n");
}

wchar_t wc;
for (wc=0; wc<=0x; wc++) {
if (!wcwidth(wc)) {
printf("%04X\n",wc);
}
}
}
import unicodedata,sys

for char in sys.stdin:
char = char[:-1]
c = unichr(int(char,16))
try:
print char, int(unicodedata.combining(c)!=0), unicodedata.name(c)
except:
print
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: horrible utf-8 performace in wc

2008-05-07 Thread Pádraig Brady
Bo Borgerson wrote:
> Pádraig Brady wrote:
>> In the first 65535 code points there are also 404 chars which are
>> not classed as combining in the unicode database, but are classed
>> as zero width in the glibc locale data at least (zero-width space
>> being one of them like you mentioned). I determined this with the
>> attached progs:
>>
>> ./zw | python unidata.py | grep " 0 " | wc -l
> 
> 
> Hi Pádraig,
> 
> Wow, I knew there were some stand-alone zero-width characters, but I had
> no idea there were so many!

I'm not sure should many of those be counted anyway.
But the combining class is all we have to go on.

> 
> I poked around a little in gnulib and found a function for determining
> the combining class of a Unicode character.
> 
> I think the attached patch does what you were intending to do, and it
> also counts all of the stand-alone zero-width characters you found:

cool, thanks.
Could you could optimize it though and do the following
as you've already calculated wcwidth().

  if (!width && uc_combining_class(wide_char))
chars--;

I did notice that wcwidth(0x1B44) returns 1 but I think that is because
this combining char is new in unicode version 5.0, and my locale tables
are probably not up to date. Search for "adeg adeg" here:
http://unicode.org/versions/Unicode5.0.0/ch11.pdf
I also notice the gnulib/uniwidth/ functions which might be more up to date
and calculate wcwidth(0x1B44) correctly as 0?

thanks again,
Pádraig


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: horrible utf-8 performace in wc

2008-05-08 Thread Pádraig Brady
Bruno Haible wrote:
> As a consequence:
>   - The number of characters is the same as the number of wide characters.
>   - "wc -m" must output the number of characters.
>   - In a Unicode locale,  is one character, and  is
> two characters,

Fair enough.

> If you want wc to count characters after canonicalization, then you can
> invent a new wc command-line option for it.

I guess one would could possibly have --chars={unicode,glyph,grapheme,column}
with unicode being the default, and how it currently works.

> But I would find it more useful
> to have a filter program that reads from standard input and writes the
> canonicalized output to standard output; that would be applicable in many
> more situations.

That would be _very_ useful, yes.

thanks for all the great info in this thread,
Pádraig.



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


[PATCH] Clarify field delimiter description in uniq --help output

2008-05-19 Thread Pádraig Brady
>From 9228e10c4c68ffd585bbb6e5fd7efbab6f6c5f97 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Mon, 19 May 2008 07:54:07 +0100
Subject: [PATCH] Clarify field delimiter description in uniq --help output

* src/uniq.c: Clarify in help output that field delimiters are blanks and
not the larger set of whitespace characters.

Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 src/uniq.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/uniq.c b/src/uniq.c
index 1a54055..3dac9ee 100644
--- a/src/uniq.c
+++ b/src/uniq.c
@@ -165,7 +165,7 @@ Mandatory arguments to long options are mandatory for short options too.\n\
  fputs (VERSION_OPTION_DESCRIPTION, stdout);
  fputs (_("\
 \n\
-A field is a run of whitespace, then non-whitespace characters.\n\
+A field is a run of spaces or tabs, then non-blank characters.\n\
 Fields are skipped before chars.\n\
 "), stdout);
  fputs (_("\
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Clarify field delimiter description in uniq --help output

2008-05-19 Thread Pádraig Brady
Jim Meyering wrote:
>> Hi Pádraig,
>>
>> Thanks!
>> Your summary is right, but the --help change is slightly
>> inaccurate, since "blank" can mean more than just SPC or TAB.

Yes you're right for non C locales.

In my en_IE.utf8 locale isblank() is the larger set of:

0009 TAB
0020 SPACE
1680 OGHAM SPACE MARK
180E MONGOLIAN VOWEL SEPARATOR
2000 EN QUAD
2001 EM QUAD
2002 EN SPACE
2003 EM SPACE
2004 THREE-PER-EM SPACE
2005 FOUR-PER-EM SPACE
2006 SIX-PER-EM SPACE
2008 PUNCTUATION SPACE
2009 THIN SPACE
200A HAIR SPACE
205F MEDIUM MATHEMATICAL SPACE
3000 IDEOGRAPHIC SPACE

It could also be argued that my "spaces or tabs" was more descriptive,
and could also imply the "spaces" above?
Personally I had to `man isblank` to see what was considered blank,
and the linux man page only mentions the space and tab chars anyway.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Clarify field delimiter description in uniq --help output

2008-05-19 Thread Pádraig Brady
Jim Meyering wrote:
> 
> I agree that more detail would be better.
> What do you think of this additional change?
> 
> -A field is a run of blanks, then non-blank characters.\n\
> -Fields are skipped before chars.\n\
> +A field is a run of blanks (usually spaces and/or TABs), then non-blank\n\
> +characters.  Fields are skipped before chars.\n\

spot on.

thanks,
Pádraig.



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: cp: performance improvement with small files

2008-05-23 Thread Pádraig Brady
Hauke Laging wrote:
> Hello,
> 
> I just read an interesting hint in the German shell Usenet group 
> (<[EMAIL PROTECTED]>). As I could not find anything about 
> that point in your mailing list archive I would like to mention it here.
> 
> The author claims that he achieved a huge performance increase (more than 
> factor 10) when copying a big amount of small files (1-10 KiB) by sorting 
> by inode numbers first. This probably reduces the disk access time which 
> becomes the dominating factor for small files.

disk seeks have mostly missed the computer performance increases over time,
and hence why they're increasing being noticed as the bottleneck.
However I think mechanical disks will quickly become a thing of the past,
with the onset of solid state disks.

I've noticed myself a large performance gain in the few filesystems I've
tested by sorting by inode, so that the disk head seeks in 1 direction only.
One can see this in the findup component of fslint here for example:
http://code.google.com/p/fslint/source/browse/tags/2.26/fslint/findup#158
I've also found though that sorting by path is only a little worse.
Sorting by md5sum is very bad though :)

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: timeout command

2008-06-02 Thread Pádraig Brady
Eric Blake wrote:
> I just noticed the addition of the timeout command:
> http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=265c4b8
> 
> But was this hunk to configure.ac intentional?  It isn't mentioned in
> ChangeLog:
> http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=265c4b8#patch4

Sorry that was unintentional.
I did a last minute change without proper testing :(
(I was being called for dinner).

BTW why does gnulib require "program_name" to be exported?
Should this be a syntax-check?

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: horrible utf-8 performace in wc

2008-06-05 Thread Pádraig Brady
There have been some interesting "counting UTF-8 strings" threads
over at reddit lately, all referenced from this article:
http://www.daemonology.net/blog/2008-06-05-faster-utf8-strlen.html

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate-fail-diag failure on Solaris

2008-06-10 Thread Pádraig Brady
Bruno Haible wrote:
> Building coreutils 6.12.29-a16be on Solaris 7, with gcc 3.2.2. It fails
> like this:
> 
> ===
>GNU coreutils 6.12.29-a16be: tests/test-suite.log   
> ===
> 
> 1 of 344 tests failed.  (42 tests were not run).  
> 
> FAIL: misc/truncate-fail-diag.log (exit: 1)
> ===

> The problem is that "truncate -s0 no/" creates a zero-sized file named 'no'
> and returns with exit code 0, but the test expects a failure.

eek. I'm surprised that solaris 7 creates a file when one specifies the 
trailing /
Solaris 10 gives an ENOTDIR error, while linux gives an EISDIR error.
I'm not sure what to do in this case. Probably just remove the second
truncate test in that file altogether.

thanks,
Pádraig.



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: misc/truncate-fail-diag, cp/thru-dangling, mkdir/selinux failures on HP-UX

2008-06-11 Thread Pádraig Brady
Bruno Haible wrote:
> Building coreutils-6.12.29-a16be on HP-UX 11.00, there are 3 failures.
> 
> FAIL: misc/truncate-fail-diag.log (exit: 1)
> ===
> 
> + dir=no/
> + truncate -s0 no/
> + 1> out 2>& 1
> + fail=1

So that's the same issue as solaris 7 had, yes?
I vote to skip the test on those systems.

case $host_triplet in
  *solaris7*|*hpux*) skip_test_ 'open does not work as expected';;
esac

Bruno, can you provide the appropriate $host_triplets.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: can you please add a synchronization feature to the GNU cp

2008-06-11 Thread Pádraig Brady
William Tambe wrote:
> Hi,
> Can you please add a synchronization feature to the GNU cp so that files
> not found in the source files/folders get removed from the destination
> folder?
> 
> This is a very useful feature for backups without having to use very
> expensive routines.
> 
> I know rsync provide a similar feature but it would be nice to have just
> a single option to the cp command to do the work without having to use
> another tool which has its own set of options and feature.

The unix way is to have specific tools for specific jobs

> find . | while read file
> do
> [ -e "/${file}" -o -L "/${file}" ] || rm -vrf ${file}
> done

shell looping is slow BTW. Always try to push iteration into compiled logic.
For example you could do instead:

export LANG=C #for speed
find . / -printf "%P\n" | sort | uniq -u | xargs rm

And that's just off the top of my head.
Please test it a million times before running.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: unable to remove duplicate lines from the attached file

2008-06-13 Thread Pádraig Brady
Joseph Butler wrote:
> 
> Hello Tech Support:
> 
> For some reason, I'm unable to remove duplicate lines using the uniq utility.
> 
> Any solution to this issue would be appreciated.

Trailing whitespace:

sed 's/[ \r]*$//' test03 | sort | uniq -d


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Multi-threading in sort(or core-utils)

2008-06-13 Thread Pádraig Brady
Bo Borgerson wrote:
> [EMAIL PROTECTED] wrote:
>> Hello
>> Few minutes ago i used sort -u for sorting big file(236 Mb). I have 2 core 
>> cpu(core 2 duo), but i found that sort use only one cpu(work in one thread). 
>> I think it is good idea to make option(or by default) for sorting in threads 
>> to increase performance on systems that might execute more than one thread 
>> in parallel.
>>Klimentov Konstantin.
> 
> Hi,
> 
> If you're using a shell that supports process substitution you could try
> splitting your file in half and putting the bulk sorts of each half as
> inputs to a merge:
> 
> So if you were doing:
> 
> $ sort bigfile
> 
> You could do:
> 
> $ sort -m <(sort bigfile.firsthalf) <(sort bigfile.secondhalf)

A few notes about that

1. `LANG=C sort ...` may be appropriate for your data, and should
be much quicker than using the collating routines for your locale

2. How to split a text file into chunks correctly is not
obvious to me. Here is a little snippet that might work:

file="$1"; chunks=2
size=$(find "$file" -printf %s)
line_fuzz=80 #to avoid single line for last chunk
chunk_max=$((($size+$line_fuzz)/$chunks))
split --line-bytes=$chunk_max "$file"

3. The file chunks if, on traditional hard disks,
should be on separate spindles. I.E. if both sort processes
are reading off the same spindle, they will be fighting over
the disk heads and just slow things down a lot.

4. If processing from a Solid State Disk, or doing multiple runs
from cache etc, it will probably be quicker to process the portions
of the file directly, without having to split them up first.
Here is a snippet to process (approximately) each half of
a text file directly:

size=$(find "$1" -printf %s)
half=$(($size/2))
next=$(dd if="$1" bs=1 skip=$half 2>/dev/null | sed q | wc -c)
chunk2_s=$(($half+$next))
#note head will read in blocks of 8192
sort -m <(head -c $chunk2_s "$1" | sort) \
<(tail -c +$(($chunk2_s+1)) "$1" | sort)

5. I think it would be nice for dd to support reading portions of
a file efficiently. As far as I can see it can only do it by reading
1 byte at a time. Perhaps skip_bytes=x and count_bytes=x would
be useful additions?

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: coreutils-6.12.29-a16be - FAIL: misc/truncate-fail-diag.log

2008-06-21 Thread Pádraig Brady
Poor Yorick wrote:
> Building latest snapshot of coreutils, coreutils-6.12.29-a16be, on SunOS 5.9,
> with gcc-4.2.2 and gnu ld:
> 
> FAIL: misc/truncate-fail-diag.log

This is a bug in SunOS which is worked around in later gnulibs.
Please try the latest snapshot just released.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: coreutils-6.12.29-a16be - FAIL: misc/truncate-fail-diag.log

2008-06-22 Thread Pádraig Brady
Poor Yorick wrote:
>>  From: Pádraig Brady <>
>>  Sent: 2008-06-21 14:19
>>  
>>  Poor Yorick wrote:
>>  > Building latest snapshot of coreutils, coreutils-6.12.29-a16be, on SunOS 
>> 5.9,
>>  > with gcc-4.2.2 and gnu ld:
>>  >
>>  > FAIL: misc/truncate-fail-diag.log
>>  
>>  This is a bug in SunOS which is worked around in later gnulibs.
>>  Please try the latest snapshot just released.
>>  
> 
> Same test failed with coreutils-6.12.66-c1aab.  truncate-fail-diag.log 
> attached.

Oops, this snapshot doesn't seem to contain the version of gnulib with the fix:
http://lists.gnu.org/archive/html/bug-coreutils/2008-06/msg00121.html
I presume it will be included soon.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-25 Thread Pádraig Brady
Michael Geng wrote:
> Hi,
> 
> When I'm running make distcheck on the Coreutils I get an error message
> 
> ...
> gcc -std=gnu99  -I. -I../lib  -I../lib   -Werror -ansi -Wno-long-long -MT 
> truncate.o -MD -MP -MF .deps/truncate.Tpo -c -o truncate.o truncate.c
> cc1: warnings being treated as errors
> truncate.c: In function 'parse_len':
> truncate.c:78: error: passing argument 4 of 'xstrtoimax' from incompatible 
> pointer type
> make[6]: *** [truncate.o] Error 1
> ...
> 
> I'm running Linux on a PC and I compiled the Coreutils from git
> from this morning bootstrapped with Gnulib also from git from
> this morning.
> 
> Is this a bug in the Coreutils or is there something wrong on my system?

What version of glibc are you using.
I make the assumption that OFF_T_MAX = INTMAX_MAX,
but this is only true when _FILE_OFFSET_BITS=64
I'll look into fixing that up this evening.
I wonder why that is not set in your lib/config.h ?

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-25 Thread Pádraig Brady
Jim Meyering wrote:
> Here's a tentative patch:
> 
> diff --git a/src/truncate.c b/src/truncate.c
> index 8febd58..52500d2 100644
> --- a/src/truncate.c
> +++ b/src/truncate.c
> @@ -74,10 +74,21 @@ static int
>  parse_len (char const *str, off_t *size)
>  {
>enum strtol_error e;
> -  /* OFF_T_MAX = INTMAX_MAX */
> -  e = xstrtoimax (str, NULL, 10, size, "EgGkKmMPtTYZ0");
> -  errno = (e == LONGINT_OVERFLOW) ? EOVERFLOW : 0;
> -  return (e == LONGINT_OK) ? 0 : -1;
> +  intmax_t tmp_size;
> +  e = xstrtoimax (str, NULL, 10, &tmp_size, "EgGkKmMPtTYZ0");
> +  if (e == LONGINT_OK
> +  && !(OFF_T_MIN <= tmp_size && tmp_size <= OFF_T_MAX))
> +e = LONGINT_OVERFLOW;
> +
> +  if (e == LONGINT_OK)
> +{
> +  errno = 0;
> +  *size = tmp_size;
> +  return 0;
> +}
> +
> +  errno = (e == LONGINT_OVERFLOW ? EOVERFLOW : 0);
> +  return -1;
>  }
> 
>  static void
> 

I reviewed all uses of off_t in the code and came up with
an almost identical patch, so I'm happy to go with the above.

Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>

Micheal there still is something wrong with your system I think.
Did you perhaps compile glibc from scratch and didn't pass
a required configure option for Large File Support?

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-25 Thread Pádraig Brady
Michael Geng wrote:
> On Wed, Jun 25, 2008 at 08:19:56PM +0100, Pádraig Brady wrote:
>> Micheal there still is something wrong with your system I think.
>> Did you perhaps compile glibc from scratch and didn't pass
>> a required configure option for Large File Support?
> 
> I didn't compile glibc myself. So I don't know what's wrong on
> my system. But never mind, thanks to Jim's quick patch it works 
> now :-)

There may be nothing wrong with your system, I'm just speculating.
For example on 64 bit systems which I don't have access to,
there may be a mismatch between intmax_t and off_t which is not
obvious to me? Anyway it would be good to confirm that you
can infact create a "large file" using: `truncate -s3G large_file`

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-26 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> Jim Meyering wrote:
>>> Here's a tentative patch:
> ...
>> I reviewed all uses of off_t in the code and came up with
>> an almost identical patch, so I'm happy to go with the above.
> 
> Hi Pádraig,
> Thanks for checking.
> Here's what I've pushed:
> [I added the IF_LINT initializer]

Hmm, it's probably cleaner to assign *size unconditionally,
as I had done in my patch, but it doesn't really matter.

I also noticed this morning that some format specifiers also
assumed sizeof(off_t) == sizeof(intmax_t) and thus would
print garbage off the stack if this wasn't the case.

Attached is my current patch for reference.

thanks,
Pádraig.
>From 6e44a9d43a9c88cb943c5583c63b2c81bbb17e64 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Wed, 25 Jun 2008 19:27:40 +0100
Subject: [PATCH] truncate: Add support for systems without large file support

* src/truncate.c: don't assume off_t is same size as intmax_t

Signed-off-by: Pádraig Brady <[EMAIL PROTECTED]>
---
 src/truncate.c |   12 
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/truncate.c b/src/truncate.c
index f26fd45..044ae4e 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -73,8 +73,12 @@ static int
 parse_len (char const *str, off_t *size)
 {
   enum strtol_error e;
-  /* OFF_T_MAX = INTMAX_MAX */
-  e = xstrtoimax (str, NULL, 10, size, "EgGkKmMPtTYZ0");
+  intmax_t tmp_size; /* OFF_T_MAX <= INTMAX_MAX */
+  e = xstrtoimax (str, NULL, 10, &tmp_size, "EgGkKmMPtTYZ0");
+  if (e == LONGINT_OK &&
+  (tmp_size < OFF_T_MIN || tmp_size > OFF_T_MAX))
+e = LONGINT_OVERFLOW;
+  *size = tmp_size;
   errno = (e == LONGINT_OVERFLOW) ? EOVERFLOW : 0;
   return (e == LONGINT_OK) ? 0 : -1;
 }
@@ -148,7 +152,7 @@ do_ftruncate (int fd, char const *fname, off_t ssize, rel_mode_t rel_mode)
 {
   error (0, 0,
  _("overflow in %" PRIdMAX
-   " * %zu byte blocks for file %s"), ssize, blksize,
+   " * %zu byte blocks for file %s"), (intmax_t) ssize, blksize,
  quote (fname));
   return 1;
 }
@@ -229,7 +233,7 @@ do_ftruncate (int fd, char const *fname, off_t ssize, rel_mode_t rel_mode)
 {
   error (0, ftruncate_errno,
  _("truncating %s at %" PRIdMAX " bytes"), quote (fname),
- nsize);
+ (intmax_t) nsize);
   return 1;
 }
   return 0;
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-26 Thread Pádraig Brady
Jim Meyering wrote:
> By the way, does %zu work reliably everywhere now?
> In the past, we've converted such values to strings via umaxtostr,
> to accommodate older systems.
> 
> For ssize and nsize, I have a slight preference for the
> cast-free approach of using %s with imaxtostr.

Yes I suppose %z and PRIdMAX are C99 specific.
How about the attached patch?

cheers,
Pádraig.
>From f06afe7a149c3d0340eeddd67718b3f7254a8251 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Thu, 26 Jun 2008 11:10:13 +0100
Subject: [PATCH] truncate: Fix portability issues with printing numbers in error messages

* src/truncate.c: Remove C99 specific format specifiers,
and explicitly convert from off_t to intmax_t which
may be a different type.
---
 src/truncate.c |   12 
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/truncate.c b/src/truncate.c
index 2435a12..f5bcfdc 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -32,6 +32,7 @@
 
 #include "system.h"
 #include "error.h"
+#include "inttostr.h"
 #include "posixver.h"
 #include "quote.h"
 #include "xstrtol.h"
@@ -158,9 +159,11 @@ do_ftruncate (int fd, char const *fname, off_t ssize, rel_mode_t rel_mode)
   size_t const blksize = ST_BLKSIZE (sb);
   if (ssize < OFF_T_MIN / blksize || ssize > OFF_T_MAX / blksize)
 {
+  char jbuf[INT_BUFSIZE_BOUND (intmax_t)];
+  char zbuf[INT_BUFSIZE_BOUND (uintmax_t)];
   error (0, 0,
- _("overflow in %" PRIdMAX
-   " * %zu byte blocks for file %s"), ssize, blksize,
+ _("overflow in %s * %s byte blocks for file %s"),
+ imaxtostr (ssize, jbuf), umaxtostr (blksize, zbuf),
  quote (fname));
   return 1;
 }
@@ -239,9 +242,10 @@ do_ftruncate (int fd, char const *fname, off_t ssize, rel_mode_t rel_mode)
   else if (S_ISREG (sb.st_mode) || S_ISDIR (sb.st_mode)
|| S_TYPEISSHM (&sb))
 {
+  char jbuf[INT_BUFSIZE_BOUND (intmax_t)];
   error (0, ftruncate_errno,
- _("truncating %s at %" PRIdMAX " bytes"), quote (fname),
- nsize);
+ _("truncating %s at %s bytes"), quote (fname),
+ imaxtostr (nsize, jbuf));
   return 1;
 }
   return 0;
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-26 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> Jim Meyering wrote:
>>> By the way, does %zu work reliably everywhere now?
>>> In the past, we've converted such values to strings via umaxtostr,
>>> to accommodate older systems.
>>>
>>> For ssize and nsize, I have a slight preference for the
>>> cast-free approach of using %s with imaxtostr.
>> Yes I suppose %z and PRIdMAX are C99 specific.
>> How about the attached patch?
> 
> Thanks!
> However, I'd prefer to keep the use of PRIdMAX,
> since gnulib's inttypes.h replacement lets us rely on that,
> if you don't mind.

I'm a little confused. Did you change your mind and you
now want to keep the (intmax_t) cast in my original patch?

I also noticed that the use of size_t is buggy anyway
in conjunction with off_t, so I've changed that.

thanks,
Pádraig.

>From ee15430cb9b0de578269262ee149aa9350184354 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Thu, 26 Jun 2008 11:10:13 +0100
Subject: [PATCH] truncate: Fix integer portability issues

* src/truncate.c: Explicitly convert from off_t to intmax_t
when printing numbers as they may be different types.
Also don't mix size_t and off_t types in operations as
the latter will be promoted to unsigned when these types
are the same size.
---
 src/truncate.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/truncate.c b/src/truncate.c
index 2435a12..02d4102 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -155,12 +155,13 @@ do_ftruncate (int fd, char const *fname, off_t ssize, rel_mode_t rel_mode)
 }
   if (block_mode)
 {
-  size_t const blksize = ST_BLKSIZE (sb);
+  off_t const blksize = ST_BLKSIZE (sb);
   if (ssize < OFF_T_MIN / blksize || ssize > OFF_T_MAX / blksize)
 {
   error (0, 0,
  _("overflow in %" PRIdMAX
-   " * %zu byte blocks for file %s"), ssize, blksize,
+   " * %" PRIdMAX " byte blocks for file %s"),
+ (intmax_t) ssize, (intmax_t) blksize,
  quote (fname));
   return 1;
 }
@@ -241,7 +242,7 @@ do_ftruncate (int fd, char const *fname, off_t ssize, rel_mode_t rel_mode)
 {
   error (0, ftruncate_errno,
  _("truncating %s at %" PRIdMAX " bytes"), quote (fname),
- nsize);
+ (intmax_t) nsize);
   return 1;
 }
   return 0;
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: 6.12.70-4f470 with 64-bit gcc and gnu ld on Solaris 9 - timeout-parameters.log

2008-06-26 Thread Pádraig Brady
Poor Yorick wrote:
> compiling 6.12.70-4f470 with 64-bit gcc (with -m64) and 64-bit gnu ld on 
> Solaris 9:

So on this system `timeout 4294967296 sleep 0` is invalid
but `timeout 49711d sleep 0` is OK?
The apply_time_suffix() function should be returning an
error for the latter case but is not.
I can't see the problem by inspection, so
I need help from someone with a 64 bit system.

I also noticed a couple of other problems in those parameter tests,
fixes for which are attached.

cheers,
Pádraig.
>From 1d974563bccae68c9b15c4b9df30b4d65bb2fb11 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Thu, 26 Jun 2008 23:09:28 +0100
Subject: [PATCH] timeout: fixup invalid parameter tests

tests/misc/timeout-parameters: remove test for invalid
signal number and I don't know what signal number are
invalid on all systems. Also tweak the other invalid
signal check so that the rest of the parameters are correct.
---
 tests/misc/timeout-parameters |5 +
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/tests/misc/timeout-parameters b/tests/misc/timeout-parameters
index 091fbd7..e44ff98 100755
--- a/tests/misc/timeout-parameters
+++ b/tests/misc/timeout-parameters
@@ -41,10 +41,7 @@ timeout 4294967296 sleep 0 && fail=1
 timeout 49711d sleep 0 && fail=1
 
 # invalid signal spec
-timeout --signal=invalid sleep 0 && fail=1
-
-# invalid signal number
-timeout --signal=128 sleep 0 && fail=1
+timeout --signal=invalid 1 sleep 0 && fail=1
 
 # invalid command
 timeout 1 . && fail=1
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-27 Thread Pádraig Brady
Rather than relying on code inspection to find places
where signed ints would be erroneously promoted to unsigned,
I went looking for a gcc option to automatically detect this.
It's -Wsign-compare which is automatically included in -Wextra.
Note it might be good to add to README-hacking to run configure
with the CFLAGS="-Wall -Wextra" options?

So turning that on didn't show any more bugs, but I've attached
a patch to fix the warnings in a couple of places where the
code was ambiguous.

There is still a warning given by the ST_BLKSIZE() macro in
src/system.h where it compares a blksize_t which is long int
on my system, to SIZE_MAX. Perhaps a cast to (size_t) is
required there to confirm the intent?

thanks,
Pádraig.
>From 9af2fe570111c4073588293facdea87cf9151e55 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Fri, 27 Jun 2008 08:38:42 +0100
Subject: [PATCH] truncate: Silence -Wsign-compare errors

src/truncate.c: Cast signed to unsigned to
confirm intent which will silence -Wsign-compare errors
---
 src/truncate.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/truncate.c b/src/truncate.c
index 02d4102..fd321c6 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -189,9 +189,9 @@ do_ftruncate (int fd, char const *fname, off_t ssize, rel_mode_t rel_mode)
 }
 
   if (rel_mode == rm_min)
-nsize = MAX (fsize, ssize);
+nsize = MAX (fsize, (uintmax_t) ssize);
   else if (rel_mode == rm_max)
-nsize = MIN (fsize, ssize);
+nsize = MIN (fsize, (uintmax_t) ssize);
   else if (rel_mode == rm_rdn)
 /* 0..ssize-1 -> 0 */
 nsize = (fsize / ssize) * ssize;
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: truncate.c fails to compile on make distcheck

2008-06-27 Thread Pádraig Brady
Paul Eggert wrote:
> Pádraig Brady <[EMAIL PROTECTED]> writes:
> 
>> diff --git a/src/truncate.c b/src/truncate.c
>> index 02d4102..fd321c6 100644
>> --- a/src/truncate.c
>> +++ b/src/truncate.c
>> @@ -189,9 +189,9 @@ do_ftruncate (int fd, char const *fname, off_t ssize, 
>> rel_mode_t rel_mode)
>>  }
>>  
>>if (rel_mode == rm_min)
>> -nsize = MAX (fsize, ssize);
>> +nsize = MAX (fsize, (uintmax_t) ssize);
>>else if (rel_mode == rm_max)
>> -nsize = MIN (fsize, ssize);
>> +nsize = MIN (fsize, (uintmax_t) ssize);
>>else if (rel_mode == rm_rdn)
>>  /* 0..ssize-1 -> 0 */
>>  nsize = (fsize / ssize) * ssize;
> 
> This patch does not look right to me.  If ssize is negative, it screws up.
> 
> Can ssize be negative?  Yes it can.

Not when rel_mode is rm_min or rm_max.

> This indicates a bug in the underlying code, which GCC was right to
> warn us about, and which we should not have worked around by inserting
> casts blindly.

As I said in the patch the casts were to confirm the intent,
rather than just to silence the errors.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


[PATCH] truncate: Ignore whitespace in --size parameters

2008-06-28 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
> 
>> Paul Eggert wrote:
>>> This indicates a bug in the underlying code, which GCC was right to
>>> warn us about, and which we should not have worked around by inserting
>>> casts blindly.
>> As I said in the patch the casts were to confirm the intent,
>> rather than just to silence the errors.
> 
> I see the intent, but we can sneak past the check
> that detects this:
> 
> $ ./truncate -s '<-9' k
> ./truncate: multiple relative modifiers specified
> Try `./truncate --help' for more information.
> 
> by adding a space before the "-":
> 
> $ ./truncate -s '< -9' k && echo whoops
> whoops

Well found! That space would mean that
`truncate -s '> -1' file` would truncate file to 0!

It also means that the more likely command of
`truncate -s " +1" would also truncate a file to 1,
rather than extending the size of file by 1 byte.

patch attached.

thanks,
Pádraig.
>From fbb9a05e83f4af8b4aa99c82c6fbad0849afd9a0 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Sun, 29 Jun 2008 01:55:03 +0100
Subject: [PATCH] truncate: Ignore whitespace in --size parameters.
 Without this `truncate -s '> -1' file` would truncate file to 0,
 and `truncate -s " +1" would truncate a file to 1 byte.

src/truncate.c: For --size parameter, skip leading whitespace
and any whitespace after one of the relative modifiers so that
the presence of the +/- modifiers can be checked for reliably.
tests/misc/truncate-parameters: Add tests for spaces in --size parameter
---
 src/truncate.c |6 ++
 tests/misc/truncate-parameters |6 ++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/src/truncate.c b/src/truncate.c
index fd321c6..3bc52ca 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -286,6 +286,9 @@ main (int argc, char **argv)
   break;
 
 case 's':
+  /* skip any whitespace */
+  while (isspace (*optarg))
+optarg++;
   switch (*optarg)
 {
 case '<':
@@ -305,6 +308,9 @@ main (int argc, char **argv)
   optarg++;
   break;
 }
+  /* skip any whitespace */
+  while (isspace (*optarg))
+optarg++;
   if (*optarg == '+' || *optarg == '-')
 {
   if (rel_mode)
diff --git a/tests/misc/truncate-parameters b/tests/misc/truncate-parameters
index e416831..d08c9b2 100755
--- a/tests/misc/truncate-parameters
+++ b/tests/misc/truncate-parameters
@@ -40,4 +40,10 @@ truncate --io-blocks --reference=file file && fail=1
 # must specify valid numbers
 truncate --size="invalid" file && fail=1
 
+# spaces not significant around size
+truncate --size="> -1" file && fail=1
+truncate --size=" >1" file || fail=1 #file now 1
+truncate --size=" +1" file || fail=1 #file now 2
+[ $(du -b file | cut -f1) -eq 2 ] || fail=1
+
 (exit $fail); exit $fail
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: dd (coreutils 6.10)

2008-06-29 Thread Pádraig Brady
Jon Grant wrote:
> Hi,
> 
> I am running dd from coreutils 6.10 on the latest Knoppix disc, trying
> to copy a failing drive to a new drive so i can try mount the partition.
> 
> I noticed that dd tries the to read the same 512-byte block 40 times
> before giving up.

In addition if dd can't read the block at all it will not
write anything to the output, and so your partitions will
be even more messed up.

ddrescue may be better sorted to your needs.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: du: cache directory size

2008-07-03 Thread Pádraig Brady
Mildred wrote:
> Hi,
> 
> I noticed that du takes a long time to scan directories and measure
> disk usage.
> 
> I was thinking that perhaps caching the size of directories could bu
> useful. Perhaps, after du computes the size of a directory, it could
> write its size in its extended attributes (if the filesystem support
> it). Next time, du would only compare the directory atime (or mtime
> perhaps) with the time of the scan. That could possibly save some time
> during the scan.

Those values would be invalidated though once a change
has been written anywhere in the tree under a directory.

> Perhaps this is not something to be included in du itself, but I
> thought it could be a good idea to give the idea anyway.

The idea might be appropriate on a per file basis.

For example, md5sum which needs to do significant processing
on all of a file's data, could cache the computed value in
an extended attribute. You would need support in the filesystem
though, to invalidate certain extended attributes on write.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


[PATCH] avoid -Wsign-compare warnings

2008-07-04 Thread Pádraig Brady
I noticed Jim fixed a couple obvious
issues highlighted by the -Wsigned-compare gcc option
we were talking about a few days ago.

The attached patch silences all other instances,
so that this option may be used to help find these
hard to spot errors, when they're introduced in future.

Note this patch is just for comment at this stage.
Also note it is not tested on 64 bit.

thanks,
Pádraig.
>From e22c8a33c74d0d5224f5b542bc366a689c3b4231 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Thu, 3 Jul 2008 19:27:18 +0100
Subject: [PATCH] avoid -Wsign-compare warnings

* src/copy.c: Cast st_size to umaxint_t in unsigned comparisons,
Use already assigned unsigned variable n, rather than signed n_read
in unsigned comparison.
* src/dd.c: Cast pointer arithmetic to unsigned, and add an assert,
to ensure this is valid. Store lseek return in uintmax_t as it's
used in various unsigned comparisions. Cast to off_t only in error check.
* src/du.c: Store st_size in uintmax_t explicitly to avoid warning.
Also add an assert, to check for the improbable overflow condition.
* src/group-list.c: Use int rather than size_t as variable is used
in signed comparisons.
* src/id.c: Likewise.
* src/ls.c: Cast widths to unsigned as always positive.
* src/od.c: Use unsigned widths as always positive.
* src/pathchk.c: Compare pathconf limits to signed MAX constants,
as pathconf returns signed values.
* src/pr.c: Use unsigned variables in unsigned comparisons.
* src/shuf.c: Likewise.
* src/ptx.c: Cast st_size to umaxint_t in unsigned comparison.
* src/shred.c: Use already assigned signed variable sizeof_r,
rather than the unsigned sizeof(r). Don't use signed integer
overflow check that comtemporary compilers may remove.
* src/sort.c: Use unsigned file_size in unsigned comparisions,
which is valid as we're now checking st.st_size >= 0.
* src/system.h: Cast st_blksize to uintmax_t as it's being compared
to an unsigned MAX constant.
* src/tac.c: Store lseek return in uintmax_t as it's used in
various unsigned comparisions. Cast to off_t only in error check.
---
 src/copy.c   |7 ---
 src/dd.c |   10 ++
 src/du.c |   11 ---
 src/group-list.c |2 +-
 src/id.c |2 +-
 src/ls.c |7 ---
 src/od.c |4 ++--
 src/pathchk.c|4 ++--
 src/pr.c |6 +++---
 src/ptx.c|4 ++--
 src/shred.c  |4 ++--
 src/shuf.c   |2 +-
 src/sort.c   |4 ++--
 src/system.h |2 +-
 src/tac.c|4 ++--
 15 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 82c6978..54a87ca 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -528,7 +528,8 @@ copy_reg (char const *src_name, char const *dst_name,
 
 	/* Do not bother with a buffer larger than the input file, plus one
 	   byte to make sure the file has not grown while reading it.  */
-	if (S_ISREG (src_open_sb.st_mode) && src_open_sb.st_size < buf_size)
+	if (S_ISREG (src_open_sb.st_mode) &&
+	(uintmax_t) src_open_sb.st_size < buf_size)
 	  buf_size = src_open_sb.st_size + 1;
 
 	/* However, stick with a block size that is a positive multiple of
@@ -621,7 +622,7 @@ copy_reg (char const *src_name, char const *dst_name,
 	last_write_made_hole = false;
 
 	/* A short read on a regular file means EOF.  */
-	if (n_read != buf_size && S_ISREG (src_open_sb.st_mode))
+	if (n != buf_size && S_ISREG (src_open_sb.st_mode))
 	  break;
 	  }
   }
@@ -1867,7 +1868,7 @@ copy_internal (char const *src_name, char const *dst_name,
 	  int saved_errno = errno;
 	  bool same_link = false;
 	  if (x->update && !new_dst && S_ISLNK (dst_sb.st_mode)
-	  && dst_sb.st_size == strlen (src_link_val))
+	  && (uintmax_t) dst_sb.st_size == strlen (src_link_val))
 	{
 	  /* See if the destination is already the desired symlink.
 		 FIXME: This behavior isn't documented, and seems wrong
diff --git a/src/dd.c b/src/dd.c
index ead9574..64b055e 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "system.h"
 #include "error.h"
@@ -848,7 +849,8 @@ parse_symbols (char const *str, struct symbol_value const *table,
 	{
 	  if (! entry->symbol[0])
 	{
-	  size_t slen = strcomma ? strcomma - str : strlen (str);
+	  assert (strcomma >= str);
+	  size_t slen = strcomma ? (size_t) (strcomma - str) : strlen (str);
 	  error (0, 0, "%s: %s", _(error_msgid),
 		 quotearg_n_style_mem (0, locale_quoting_style, str, slen));
 	  usage (EXIT_FAILURE);
@@ -1232,7 +1234,7 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
be seekable.  */
 
 static bool
-advance_input_after_read_error (size_t nbytes)
+advance_input_after_read_error (off_t nbytes)
 {
   if (! input_seekable)
 {
@@ -1242,7 +1244,7 @@ advance_input_after_read_error (size_t nbytes)
 }
   else
 {
-  off_t offset;
+  uintmax_t o

Re: [PATCH] avoid -Wsign-compare warnings

2008-07-11 Thread Pádraig Brady
Pádraig Brady wrote:
> I noticed Jim fixed a couple obvious
> issues highlighted by the -Wsigned-compare gcc option
> we were talking about a few days ago.
> 
> The attached patch silences all other instances,
> so that this option may be used to help find these
> hard to spot errors, when they're introduced in future.
> 
> Note this patch is just for comment at this stage.
> Also note it is not tested on 64 bit.

Thinking more about this, shows a lot of casts are
to address the gcc warning:
"signed and unsigned type in conditional expression"

I don't see why gcc is giving this warning, as
there is no comparison between signed and unsigned here.
For example in the following program compiled
with -Wsign-compare why does the second assignment
give a warning, while the first doesn't?

#include 
#include 

int main(void)
{
int i=0;
unsigned u=UINT_MAX;

if (u)  i = u; /* no warning */
i = u ? u : i; /* warning with -Wsign-compare */

printf("%d\n", i);

return 0;
}


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


[PATCH] Add expand test for lines starting with both spaces and tabs

2008-07-11 Thread Pádraig Brady
This issue is still present on fedora 9 at least,
which you can confirm with this command:

printf " \tif\n" | expand --initial -t4 |
grep -qF "$(printf '\t')" && echo buggy

So it's worth adding a test I think.

cheers,
Pádraig.
>From b36ea40fc24c6d3aca6094de90657e067fa80ff6 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Thu, 10 Jul 2008 09:36:49 +0100
Subject: [PATCH] Add expand test for lines starting with both spaces and tabs.

The expand released in current distributions (Fedora Core 4 - Fedora 9
at least), doesn't expand --initial tabs if spaces are present.

tests/misc/expand: Add test to verify --initial works correctly
with lines starting with both spaces and tabs.
---
 tests/misc/expand |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tests/misc/expand b/tests/misc/expand
index b386737..468fdee 100755
--- a/tests/misc/expand
+++ b/tests/misc/expand
@@ -28,6 +28,7 @@ my @Tests =
['t1', '--tabs=3', {IN=>"a\tb"}, {OUT=>"a  b"}],
['t2', '--tabs=3,6,9', {IN=>"a\tb\tc\td\te"}, {OUT=>"a  b  c  d e"}],
['i1', '--tabs=3 -i', {IN=>"\ta\tb"}, {OUT=>"   a\tb"}],
+   ['i2', '--tabs=3 -i', {IN=>" \ta\tb"}, {OUT=>"   a\tb"}],
   );
 
 my $save_temps = $ENV{DEBUG};
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: du: cache directory size

2008-07-16 Thread Pádraig Brady
Pádraig Brady wrote:
> Mildred wrote:
>> Hi,
>>
>> I noticed that du takes a long time to scan directories and measure
>> disk usage.
>>
>> I was thinking that perhaps caching the size of directories could bu
>> useful. Perhaps, after du computes the size of a directory, it could
>> write its size in its extended attributes (if the filesystem support
>> it). Next time, du would only compare the directory atime (or mtime
>> perhaps) with the time of the scan. That could possibly save some time
>> during the scan.
> 
> Those values would be invalidated though once a change
> has been written anywhere in the tree under a directory.

I just noticed the announcement of the Ceph distributed file system
which does maintain tree sizes for each node:

http://lkml.org/lkml/2008/7/15/385



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] dd - support for reading of full blocks

2008-07-17 Thread Pádraig Brady
Kamil Dudka wrote:
> Hello,
> 
> as solution to rhbz #431997 and #449263 I propose patch for dd - support for 
> reading of full blocks. This support is activated with dd parameter 
> conv=fullblk. This patch has no effect if parameter conv=fullblk is omitted.

This feature makes a lot of sense.
I'm not sure I like the conv=fullblk syntax though.
Here are alternatives in my order of preference:

iflag=block
conv=fill
conv=fillblock
conv=fullblock

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] dd - support for reading of full blocks

2008-07-17 Thread Pádraig Brady
Pádraig Brady wrote:
> Kamil Dudka wrote:
>> Hello,
>>
>> as solution to rhbz #431997 and #449263 I propose patch for dd - support for 
>> reading of full blocks. This support is activated with dd parameter 
>> conv=fullblk. This patch has no effect if parameter conv=fullblk is omitted.
> 
> This feature makes a lot of sense.
> I'm not sure I like the conv=fullblk syntax though.
> Here are alternatives in my order of preference:
> 
> iflag=block
> conv=fill
> conv=fillblock
> conv=fullblock

or conv=wait, though I do have a strong preference
for iflag=block as it's the most accurate.

Also would there be any possibility of making this the default?
I suppose this could break scripts that depend on short reads
from non files. Without backwards compatibility issues I would
definitely have this as the default mode of operation.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] dd - support for reading of full blocks

2008-07-18 Thread Pádraig Brady
Paul Eggert wrote:
> Pádraig Brady <[EMAIL PROTECTED]> writes:
> 
>> or conv=wait, though I do have a strong preference
>> for iflag=block as it's the most accurate.
> 
> I dunno, iflag= is supposed to be symmetric with oflag=.

Well there is no harm in having that oflag as well
to handle short writes, even though it's probably redundant.

> This "feels" more like a conversion, so conv= seems more
> appropriate.

Hmm, there is no obvious right option for me given
the current syntax of dd.
To me, conv should mean that the data is actually
changed in some way. Unfortunately that doesn't seem
to be the case currently as we have the following
conversions which actually modify the data:

ascii   from EBCDIC to ASCII
ebcdic  from ASCII to EBCDIC
ibm from ASCII to alternate EBCDIC
block   pad \n-terminated records with spaces to cbs-size
unblock replace trailing spaces in cbs-size records with \n
lcase   change upper case to lower case
ucase   change lower case to upper case
swabswap every pair of input bytes
syncpad every input block with NULs to ibs-size;
when used with block or unblock, pad with spaces

But we also have these, which should be flags IMHO:

noerror continue after read errors
notrunc do not truncate the output file
nocreat do not create the output file
exclfail if the output file already exists
fdatasync physically write output  file  data  before  finishing
fsync   likewise, but also write metadata

Note only noerror and notrunc are specified by POSIX,
and POSIX dd has no flag options at all, so it's excused :)

I do agree that a flag=block is confusing with the existing
conv=block option, so I'm currently 60/40 for iflag=wait/conv=fill

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH 1/2] Support arbitrarily-long numbers with GNU MP.

2008-07-30 Thread Pádraig Brady
Paul Eggert wrote:
> "James Youngman" <[EMAIL PROTECTED]> writes:
> 
>> Although libgmp itself has no further dependencies other than the C
>> library, this is still a large-ish extra dependency.   Should we also
>> introduce a --without-gmp option to configure?
> 
> Yes, particularly if it's added to 'expr'.
> 
>> I also remember Jim mentioning something about supporting large
>> numbers in expr.  That seems feasible, though based on the number of
>> discussions over the last year or two I would suggest that perhaps
>> using GMP in "seq" might also be a win; thoughts?
> 
> They'd both be wins, I'd say.
> 
> Also "test", right?  Though that's lower priority.

And possibly seq



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Suggestion for tr / transliteration binutil

2008-08-27 Thread Pádraig Brady
smu johnson wrote:
> Hi,
> 
> I hope this is the right place to ask this.  I think a very handy argument
> to add to the list of aliases for character classes in tr would be a new one
> that behaves like [:cntrl:], except that it will not include newline
> characters in that class.
> 
> The reason is at work here I deal with XML data that people send us that has
> all sorts of strange characters in it, like ^@, ^Z, etc etc... and I'd like
> to use tr to quickly trip all this garbage out of there without it
> destroying the newline characters too...

This is not supported at present, but you may be able to get what you want with:
tr -c '[:print:]\n' .

Your suggestion has some merit though.
I've previously needed to do something similar:
http://code.google.com/p/fslint/source/browse/trunk/fslint-gui#122

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: uniq works not correct

2008-09-10 Thread Pádraig Brady
Same error happens on fedora.

So it's probably related to that horrible i18n patch applied
by distros but rightly used the upstream coreutils project.

$ file test
test: ISO-8859 text

$ echo $LANG
en_IE.UTF-8

$ rpm -q coreutils
coreutils-6.9-12.fc8

$ ~/git/coreutils/src/uniq --version
uniq (GNU coreutils) 6.12.72-b1a1c-dirty

$ sort test test|uniq -u|wc -l
30

$ LANG=C sort test test|uniq -u|wc -l
26

$ sort test test|LANG=C uniq -u|wc -l
0

$ ~/git/coreutils/src/sort test test|~/git/coreutils/src/uniq -u|wc -l
0



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] tests: use our new timeout program

2008-10-01 Thread Pádraig Brady
Jim Meyering wrote:
> I suspended a 'make check' run, and when I resumed it I was not
> surprised to see this particular test fail.
> Then I kicked myself for not thinking to use coreutils' own
> new timeout program before this.

I presume you mean suspended the machine here, rather than
suspended the job? If just suspending the job, timeout will
still fail as the alarm will keep counting down in the kernel
(and will be sent to timeout when it's running again).

I.E. timeout currently doesn't handle a SIGSTOP or SIGTSTP specially,
as I was thinking it should count down system running time rather
than job running time, as that is dependent on many factors.
Is this correct?

Using timeout is a better solution in any case,
as it will terminate the rm command more quickly.

thanks,
Pádraig.

p.s. don't all posix shells support $((..))
I.E. $(($(date +%s) - start) rather than using $(expr $(date +%s) - $start)
I.E. if we're relying on $() shouldn't we also use $(()) ?


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-07 Thread Pádraig Brady
Giuseppe Scrivano wrote:
> Hello,
> 
> what do you think about the following way to remove the sigs_to_ignore
> hack in the timeout.c file?
> It ignores temporarily the signal inside the `send_sig' function instead
> of using the `sigs_to_ignore' array.

I'll test this tonight, as this stuff is tricky.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: cut -b on huge files

2008-10-09 Thread Pádraig Brady
Klein, Roger wrote:
> Hi,
>  
> Now I found a hint on the Web
> (http://www.programmersheaven.com/mb/linux/187697/245244/re-how-to-chang
> e-filesize-in-linux/?S=B2)

using `cut` to remove both truncate and "unsparse" files
is both inefficient an buggy. From the cut man page:

"cut - remove sections from each line of files"

The important word here is _line_
I.E. if there are any \n characters in the file
then the byte counts will be reset and
you'll get a bigger file than expected.

I suggest you do:

head --bytes=58101760 boot_image.clone2fs > boot_image.clone2fs_correct

cheers,
Pádraig.



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-09 Thread Pádraig Brady
Pádraig Brady wrote:
> Giuseppe Scrivano wrote:
>> Hello,
>>
>> what do you think about the following way to remove the sigs_to_ignore
>> hack in the timeout.c file?
>> It ignores temporarily the signal inside the `send_sig' function instead
>> of using the `sigs_to_ignore' array.
> 
> I'll test this tonight, as this stuff is tricky.

I tested it and couldn't break it,
though I'm worried about this change.

The code it replaces is very simple and explicit.
Whereas I'm worried about the implicit behaviour
of the new code. Specifically I'm worried about
races, where the monitor process may not receive
(and ignore) the signal before the signal handler
is reinstated. I.E. are we always guaranteed that
kill() is synchronous and will not return until
all signals have been delivered to all members of group?

thanks,
Pádraig,


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Pádraig Brady
Giuseppe Scrivano wrote:
> Hi Pádraig,
> 
> I think that the only problem can raise when the sent signal is received
> by the monitor process after the handler is reinstalled.  In that case
> the signal will be dispatched again to the process group.  This can
> repeat again and again until it is finally ignored by the monitor process.
> 
> On http://www.opengroup.org/onlinepubs/009695399/functions/kill.html I
> can see in the "Rationale" section:
>  "There was initially strong sentiment to specify that, if pid
>   specifies that a signal be sent to the calling process and that
>   signal is not blocked, that signal would be delivered before kill()
>   returns. This would permit a process to call kill() and be guaranteed
>   that the call never return. .. Such modifications have the effect
>   of satisfying the stronger requirement, at least when sigaction() is
>   used, but not necessarily when signal() is used."
> 
> So I guess the case I described before shouldn't happen.

Well found! From there:

Implementors are encouraged to meet the stronger requirement
[to deliver signal to calling process before kill() returns] whenever possible

So it's probably fine, and seems so on Linux from my testing,
but I don't think we should change it as I can't see any advantage.
I'm worried about (older) platforms that don't implement this _recommendation_

What I will do is remove the FIXME comment and replace it with your comments.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Pádraig Brady
Eric Blake wrote:
> On the other hand, POSIX is explicit that mixing signal and sigaction is
> not portable.  For that matter, now that gnulib provides a guaranteed
> sigaction, why don't we just change all of coreutils to use it?  Affected
> are: csplit, dd, install, ls, nohup, sort, tee, and timeout.

Good idea.

Perhaps we should just define our own signal()
or bsd_signal() that calls sigaction() appropriately.
I find signal() a much simpler interface than sigaction().




___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [PATCH] Replacement for the sigs_to_ignore hack in timeout.c

2008-10-10 Thread Pádraig Brady
Giuseppe Scrivano wrote:
> Hello,
> 
> do you have suggestions on this patch?  It replaces any `signal' with
> `sigaction'.

[snip]

> diff --git a/src/timeout.c b/src/timeout.c
> index 8b506f0..6ef7218 100644
> --- a/src/timeout.c
> +++ b/src/timeout.c
> @@ -278,8 +278,6 @@ main (int argc, char **argv)
>/* Setup handlers before fork() so that we
>   handle any signals caused by child, without races.  */
>install_signal_handlers ();
> -  signal (SIGTTIN, SIG_IGN);/* don't sTop if background child needs tty. 
>  */
> -  signal (SIGTTOU, SIG_IGN);/* don't sTop if background child needs tty. 
>  */

Moving these after the fork() introduces a race
I think as per the comment.

> +  struct sigaction sa_ignore;
> +
> +  memset (&sa_ignore, 0, sizeof sa_ignore);

Note since C90 this is equivalent to memset(0):
struct sigaction sa_ignore = {0,};

> +  sigemptyset (&sa_ignore.sa_mask);
> +  sa_ignore.sa_handler = SIG_IGN;
> +
> +  sigaction (SIGTTIN, &sa_ignore, NULL); /* don't sTop if background 
> child needs tty.  */
> +  sigaction (SIGTTOU, &sa_ignore, NULL);/* don't sTop if background 
> child needs tty.  */

As I mentioned previously it might be better
for us to define our own function that calls sigaction,
but has the same interface as signal() as this is
a much cleaner interface and usually all that's required.

thanks,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [coreutils] coreutils-7.0 expr exposes long-standing bug in matlab startup script

2008-10-14 Thread Pádraig Brady
Eric Blake wrote:
> According to Pádraig Brady on 10/14/2008 3:47 AM:
>>> With native versions of expr, and expr from GNU coreutils prior to
>>> version 7.0, the expansion of lscmd usually begins "-rw-r--r--", but
>>> the leading hyphen does not cause the word to be treated as an option.
>> I think that was an unintended change introduced when adding getopt to expr:
>> http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=f65cafd67b33009d23f940968bbe7f9a08d6fe13
> 
> Unintended or not, getopt's side effect of complaining about unrecognized
> options is necessary for standard compliance (ie. all earlier versions of
> coreutils had a bug in this area), so we should not revert it.

Quoting the spec:

"Also note that this volume of IEEE Std 1003.1-2001 permits implementations to 
extend utilities.
The expr utility permits the integer arguments to be preceded with a unary 
minus. This means that
an integer argument could look like an option. Therefore, the conforming 
application must employ
the "--" construct of Guideline 10 of the Base Definitions volume of IEEE Std 
1003.1-2001,
Section 12.2, Utility Syntax Guidelines to protect its operands if there is any 
chance the
first operand might be a negative integer (or any string with a leading minus)."

Does the above mean expr must support -- or that apps must use --
I think it's the former, and this is what the expr code tries to do.
I.E. it does try to treat non options starting with - as part of the expression.
However it gets it wrong in the case of chained short args as seen here:


localhost:~/git/coreutils/src$ expr -oh : -oh
3
localhost:~/git/coreutils/src$ expr -- -oh : -oh
3
localhost:~/git/coreutils/src$ ./expr-new -oh : -oh
./expr: syntax error
localhost:~/git/coreutils/src$ ./expr-new -o : -o
2
localhost:~/git/coreutils/src$ ./expr-new -- -oh : -oh
3


cheers,
Pádraig.

p.s. why does expr need the --bignum option anyway?
Shouldn't we just use it by default if available.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Patch to check for required programs when building from source checkout

2008-10-14 Thread Pádraig Brady
Jim Meyering wrote:
> Ed Avis <[EMAIL PROTECTED]> wrote:
>> Jim Meyering  meyering.net> writes:
>>
 A few tools are required to build coreutils from a git checkout, but
 not checked in a friendly way.
>>> The newer automake-1.10a is actually required.
>> Ah, ok, it could do with a comment because at first it appears that 1.10.1
>> should be newer than 1.10a.
> 
> That's a common misconception.
> When in doubt, use sort -V from the latest coreutils:
> 
> $ printf 'automake-1.10%s\n' .1 a|sort -V
> automake-1.10.1
> automake-1.10a
> 
> That demonstrates 1.10a is considered the newer version.

Just some notes on the dependencies are they're currently
a little bleeding edge. This stuff is second nature to me,
though I know it gives lots of people conniptions.

Note I try to keep everything on my system packaged,
but automake-1.10a is not released so you need to get from git.
So here is a log of how I just built the latest coreutils repo
on my Fedora 8 machine:

$ git clone git://git.sv.gnu.org/automake.git
$ (cd automake && ./configure --prefix=$HOME/coreutils/deps)
 configure: error: Autoconf 2.61a-341 or better is required.
# rpmbuild --rebuild 
http://download.fedora.redhat.com/pub/fedora/linux/development/source/SRPMS/autoconf-2.63-1.fc10.src.rpm
 emacs is needed by autoconf-2.63-1.fc8.noarch
 https://bugzilla.redhat.com/show_bug.cgi?id=79031
# yum install emacs #20MB
# rpmbuild --rebuild 
http://download.fedora.redhat.com/pub/fedora/linux/development/source/SRPMS/autoconf-2.63-1.fc10.src.rpm
# rpm -Uvh /usr/src/redhat/RPMS/noarch/autoconf-2.63-1.fc8.noarch.rpm
$ (cd automake && make install)
  WARNING: `help2man' is missing on your system.  You should only need it if
  make[1]: *** [automake-1.10a.1] Error 1
# yum install help2man
$ (cd automake && make install)
$ cd coreutils
$ rm -Rf gnulib #pickup latest gnulib
$ ./bootstrap
$ ./configure
$ make

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [coreutils] coreutils-7.0 expr exposes long-standing bug in matlab startup script

2008-10-14 Thread Pádraig Brady
Nelson H. F. Beebe wrote:
> I've just filed a bug report with The Mathworks, vendor of the
> widely-used Matlab matrix algebra system.  Their bin/matlab startup
> script (all versions up to current) contains code like this:
> 
>   if [ `expr "$lscmd" : '.*->.*'` -ne 0 ]; then
> 
> With native versions of expr, and expr from GNU coreutils prior to
> version 7.0, the expansion of lscmd usually begins "-rw-r--r--", but
> the leading hyphen does not cause the word to be treated as an option.

I think that was an unintended change introduced when adding getopt to expr:
http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=f65cafd67b33009d23f940968bbe7f9a08d6fe13

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [coreutils] Re: expr option interpretation changes

2008-10-14 Thread Pádraig Brady
Nelson H. F. Beebe wrote:
> I agree with a previous poster that the altered behaviour of expr in
> coreutils-7.0 as a result of using getopt() for option parsing is
> acceptable, and consistent with many other utilities, and with GNU and
> POSIX interpretation of a double-hyphen option.

I don't agree

> Here is another problem area with expr (in general, not just the GNU
> version).  Consider these two similar cases:
> 
>   expr -1 + 3
>   expr ' -1' + 3
> 
> To a human, both of these should evaluate to 2.
> 
> Apple Mac OS X, DEC Alpha OSF/1, OpenBSD, Sun Solaris 10, SGI IRIX
> native expr, and GNU/coreutils 7.0 produce 2 for the first, but
> complain "expr: non-numeric argument:" for the second.

That's what I would expect.
Note that the old and current coreutils produce the same
output for the examples above. But expanding it slightly
shows where the changes in coreutils 7.0 are questionable:

$ ./expr-7.0 -11 + 3
./expr: syntax error
$ expr-6.9 -11 + 3
-8

[minix & *BSD madness elided]

thanks for checking all these systems BTW.
That's great info to have.

I didn't post a patch as James may like to address this.
I'll post something tomorrow if not.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


[PATCH] Add option processing tests for 'expr'

2008-10-15 Thread Pádraig Brady
The attached tests pass with both your and Paul's patches.

I didn't add --bignum into the tests as I assumed
those options are being removed imminently.

cheers,
Pádraig.
>From cc29aaeaa6f1d22199abc6eb2e94a62fab46a8f5 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Wed, 15 Oct 2008 15:01:43 +0100
Subject: [PATCH] Add option processing tests for 'expr'

* tests/misc/expr: Add tests for various combinations
of options where the first part of the expression
could be confused with an option.
---
 tests/misc/expr |   11 +--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/misc/expr b/tests/misc/expr
index 4d23662..62e8ccb 100755
--- a/tests/misc/expr
+++ b/tests/misc/expr
@@ -39,6 +39,15 @@ my @Tests =
  ['f', '3 + -2', {OUT => '1'}],
  ['g', '-2 + -2', {OUT => '-4'}],
 
+ # Verify option processing.
+ # Added when option processing broke in the 7.0 beta release
+ ['opt1', '-- -11 + 12', {OUT => '1'}],
+ ['opt2', '-11 + 12', {OUT => '1'}],
+ ['opt3', '-- -1 + 2', {OUT => '1'}],
+ ['opt4', '-1 + 2', {OUT => '1'}],
+ # This evoked a syntax error diagnostic before 2.0.12.
+ ['opt5', '-- 2 + 2', {OUT => '4'}],
+
  ['paren1', '\( 100 % 6 \)', {OUT => '4'}],
  ['paren2', '\( 100 % 6 \) - 8', {OUT => '-4'}],
  ['paren3', '9 / \( 100 % 6 \) - 8', {OUT => '-6'}],
@@ -59,8 +68,6 @@ my @Tests =
  # In 5.1.3 and earlier, this would output the empty string.
  ['orempty', '"" \| ""', {OUT => '0'}, {EXIT => 1}],
 
- # This evoked a syntax error diagnostic before 2.0.12.
- ['minus2', '-- 2 + 2', {OUT => '4'}],
 
  # This erroneously succeeded and output `3' before 2.0.12.
  ['fail-a', '3 + -', {ERR => "$prog: non-numeric argument\n"},
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [coreutils] coreutils-7.0 expr exposes long-standing bug in matlab startup script

2008-10-15 Thread Pádraig Brady
Jim Meyering wrote:
> For reference, here's a small patch I just wrote that solves
> at least some of the problems reported here, while retaining
> the ability to use MP at run time.

I'd essentially done the same thing, so I think that will solve
most of the backwards compat issues.

Note if leaving the --bignum options you should probably also do:

-Usage: %s EXPRESSION\n\
-  or:  %s OPTION\n\
+Usage: %s [OPTION] EXPRESSION\n\

Though I strongly agree with Paul that it shouldn't be an option.
I was cooking up a patch as I also noticed the 64 bit int regression:

$ expr.orig 4294967296 + 1
4294967297
$ ./expr --no-bignum 4294967296 + 1
./expr: 4294967296: Numerical result out of range

But he's faster than me so less testing for me to do :)
I'll add some extra tests for the "-11 + 3" case etc.
if I don't notice any going in in the next while.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: [coreutils] coreutils-7.0 expr exposes long-standing bug in matlab startup script

2008-10-16 Thread Pádraig Brady
Excellent stuff. You got a net reduction of 366 lines,
and another 91 lines are not used in the common case
where GMP is available.

I'm a bit worried by signed overflow checking in the following.
I know gcc won't do it's ignore undefined signed overflow
code shenanigans with this, but does this assume 2s compliment
architectures? If so is this OK?

+static void
+mpz_add (mpz_t r, mpz_t a0, mpz_t b0)
+{
+  intmax_t a = a0[0];
+  intmax_t b = b0[0];
+  intmax_t val = a + b;
+  if ((val < a) != (b < 0))
+integer_overflow ('+');

What about the INT_MIN/-1 exception case in the following.
Oh I see it's short circuited in that case.
This code deserves an award for brevity or something :)

+mpz_mul (mpz_t r, mpz_t a0, mpz_t b0)
+{
+  intmax_t a = a0[0];
+  intmax_t b = b0[0];
+  intmax_t val = a * b;
+  if (! (a == 0 || b == 0
+|| ((val < 0) == ((a < 0) ^ (b < 0)) && val / a == b)))
+integer_overflow ('*');

I've attached a patch to standardize the format of the
AUTHORS define to that used in other utils with multiple authors.
I've also added you to the list since you basically rewrote
expr with this patch.

cheers,
Pádraig.
>From 057e83d69a6b212e6aa3c0943065609ab06877c5 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Thu, 16 Oct 2008 19:09:59 +0100
Subject: [PATCH] expr: Fixup authors

* src/expr.c: Standardise the format of AUTHORS to
that used in other utils with multiple authors.
Also add Paul Eggert since he basically rewrote it
with his bignum fixes.
* AUTHORS (expr): Add Paul Eggert.
---
 AUTHORS|2 +-
 src/expr.c |5 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 9c3d990..fa3c029 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -25,7 +25,7 @@ du: Torbjörn Granlund, David MacKenzie, Paul Eggert, Jim Meyering
 echo: Brian Fox, Chet Ramey
 env: Richard Mlynarik, David MacKenzie
 expand: David MacKenzie
-expr: Mike Parker, James Youngman
+expr: Mike Parker, James Youngman, Paul Eggert
 factor: Paul Rubin
 false: Jim Meyering
 fmt: Ross Paterson
diff --git a/src/expr.c b/src/expr.c
index de7a63d..65e74af 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -145,7 +145,10 @@ mpz_out_str (FILE *stream, int base, mpz_t z)
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "expr"
 
-#define AUTHORS proper_name ("Mike Parker"), proper_name ("James Youngman")
+#define AUTHORS \
+  proper_name ("Mike Parker"), \
+  proper_name ("James Youngman"), \
+  proper_name ("Paul Eggert")
 
 /* Exit statuses.  */
 enum
-- 
1.5.3.6

___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Patch to check for required programs when building from source checkout

2008-10-17 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> Jim Meyering wrote:
>>> Thanks for writing that up.
>>> Would you care to ensconce it somewhere more permanent?
>> How about the attached?
> 
> Thanks!
> However, since your instructions are Fedora 8-specific (and hence
> guaranteed to be far less useful in a year or two), I hesitate to
> put it in this file.

Well they're Fedora specific instructions rather than Fedora 8 specific.
But I take your point in that it's not general enough for that file.

> Also, you probably didn't realize, but that file
> deliberately minimizes mentions of coreutils, because I've used it almost
> verbatim in a couple other projects, e.g., idutils.  The only varying
> part is the list of required tools.

I missed that yes.

> And even that, ... prompted by recent suggestions from Ed Avis, I've been
> thinking about how best to do what he wants (bootstrap-time version/prereq
> tests) without compromising generality.  Looking at README-hacking,
> I think I found a good way:
> 
> have README-hacking refer to a list of files with package-name,min-version
> pairs and have bootstrap read that same file.  Then bootstrap would
> be able to perform tests for the required tools, and README-hacking
> would no longer have the per-package-varying list of prerequisites.
> 
> So, whether the list of pairs would go in bootstrap.cfg
> or merely be in a separate file and be read by bootstrap.cfg
> (former sounds better), I'm not sure...
> 
> Are you interested in doing something like that?

Well all missing dependencies are currently reported I think,
but it can be minutes into the build before this happens.
I'll add a list to bootstrap.conf to give immediate feedback.

> Regarding your build recipe, maybe we need a wiki?
> New file?

Perhaps we need a README-{fedora,debian,solaris,mingw,...}
./bootstrap should just work, but the above files would
detail how best to get the prerequisites.

Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: Patch to check for required programs when building from source checkout

2008-10-21 Thread Pádraig Brady
Jim Meyering wrote:
> Pádraig Brady <[EMAIL PROTECTED]> wrote:
>> Jim Meyering wrote:
> ...
>>> Are you interested in doing something like that?
>> Well all missing dependencies are currently reported I think,
>> but it can be minutes into the build before this happens.
>> I'll add a list to bootstrap.conf to give immediate feedback.
> 
> Great!
> 
>>> Regarding your build recipe, maybe we need a wiki?
>>> New file?
>> Perhaps we need a README-{fedora,debian,solaris,mingw,...}
>> ./bootstrap should just work, but the above files would
>> detail how best to get the prerequisites.
> 
> Or a README-prereq, and put them all in there?
> Either one is fine with me.

How about the attached.

Note it only checks for versions >= specifed
I.E. you can specify a min version, not a max.

Note also I was only guessing at specific versions
configured in bootstrap.conf, and specified most as
don't care.

cheers,
Pádraig.
>From 5072e68adb3b4ac462e45efc416fc10188d59fbe Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Tue, 21 Oct 2008 11:56:39 +0100
Subject: [PATCH] Add better checks and docs for build tools

* README-hacking: Organise LZMA and Valgrind as
as optional requirements rather than in their own sections.
Mention bootstrap will now check tool versions.
* README-prereq: Make a start on specific instructions
for optaining build tools. We've just notes on
Fedora linux at present.
* bootstrap.conf: Add the list of tools and versions required.
* bootstrap: Add the logic to check the for the required tools,
and list all required tools and versions if any are missing.
---
 README-hacking |   19 +++-
 README-prereq  |   30 
 bootstrap  |   81 
 bootstrap.conf |   14 +
 4 files changed, 136 insertions(+), 8 deletions(-)
 create mode 100644 README-prereq

diff --git a/README-hacking b/README-hacking
index 2e3c83a..07c87b7 100644
--- a/README-hacking
+++ b/README-hacking
@@ -8,8 +8,8 @@ These requirements do not apply when building from a distribution tarball.
 We've opted to keep only the highest-level sources in the GIT repository.
 This eases our maintenance burden, (fewer merges etc.), but imposes more
 requirements on anyone wishing to build from the just-checked-out sources.
-For example, you have to use the latest stable versions of the maintainer
-tools we depend upon, including:
+Specific tools and versions will be checked for and listed by the
+bootstrap script shown below, and will include:
 
 - Automake <http://www.gnu.org/software/automake/>
 - Autoconf <http://www.gnu.org/software/autoconf/>
@@ -22,13 +22,15 @@ tools we depend upon, including:
 - Rsync <http://samba.anu.edu.au/rsync/>
 - Tar <http://www.gnu.org/software/tar/>
 
-Valgrind <http://valgrind.org/> is also highly recommended, if
-Valgrind supports your architecture.
-
 Only building the initial full source tree will be a bit painful.
 Later, a plain `git pull && make' should be sufficient.
 
-* LZMA
+- Valgrind
+
+Valgrind <http://valgrind.org/> is also highly recommended, if
+Valgrind supports your architecture. See also README-valgrind.
+
+- LZMA
 
 This package's build procedure uses LZMA to create a compressed
 distribution tarball.  Using this feature of Automake requires
@@ -41,9 +43,10 @@ from <http://tukaani.org/lzma/>.
 You can get a copy of the source repository like this:
 
 	$ git clone git://git.sv.gnu.org/coreutils
+$ cd coreutils
 
-The next step is to get other files needed to build, which are
-extracted from other source packages:
+The next step is to get and check other files needed to build,
+which are extracted from other source packages:
 
 	$ ./bootstrap
 
diff --git a/README-prereq b/README-prereq
new file mode 100644
index 000..6ee8c34
--- /dev/null
+++ b/README-prereq
@@ -0,0 +1,30 @@
+Sometimes even the latest stable versions of certain tools do not suffice
+thus requiring you get them directly from the repository and build them
+to a location available to coreutils. Detailed below are concrete examples
+for getting the prerequisites for particular systems.
+
+- linux - fedora
+
+  This details obtaining the required tools to build coreutils 7.0
+  on a Fedora 8 system. We try to use official packages where possible:
+
+  Make sure offical git is installed
+# yum install git
+
+  The distro autoconf is too old, but there is a newer one available
+  so we rebuild that and make it available to the full system:
+# yum install emacs #autoconf build requires emacs (20MB)
+# rpmbuild --rebuild http://download.fedora.redhat.com/pub/fedora/linux/development/source/SRPMS/autoconf-2.63-1.fc10.src.rpm
+# rpm -Uvh /usr/src/redhat/RPMS/noarch/autoconf-2.63-1.fc8.noarch.rpm
+  Apply t

Re: Patch to check for required programs when building from source checkout

2008-10-21 Thread Pádraig Brady
Version 2 of bootstrap requirements checking patch attached.

Note if one wanted to fully support all (old) version formats
as automake does, one could normalize them before comparison
with something like the following:

normalize_version() {
ver="$1"

echo "$ver" | sed '
  s/^\([0-9]\{,\}\)\.\([0-9]\{,\}\)[.0]*$/\1.\2.0/; #1.10  -> 1.10.0
  s/^\([0-9]\{,\}\)\.\([0-9]\{,\}\)\([a-z]\)/\1.\2.99\3/;   #1.10a -> 
1.10.99a
  s/^\([0-9]\{,\}\)\.\([0-9]\{,\}\)-p\([0-9].*\)/\1.\2.\3/; #1.10-p1a -> 
1.10.1a
'
}

I don't think the extra complexity is worth it though.

cheers,
Pádraig.
>From 80305d49505b7383123afe0ce0c2f8e073f96afd Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Tue, 21 Oct 2008 22:40:12 +0100
Subject: [PATCH] Add better checks and docs for build tools

Prompted by a report from Ed Avis:


* README-hacking: Organise LZMA and Valgrind as
as optional requirements rather than in their own sections.
Mention bootstrap will now check tool versions.
* README-prereq: Make a start on specific instructions
for optaining build tools. Currently we just have notes
for Fedora linux.
* bootstrap.conf: Add the list of tools and versions required.
* bootstrap: Add the logic to check for the required tools,
and list all required tools and versions if any are missing.
---
 README-hacking |   35 ++-
 README-prereq  |   30 +++
 bootstrap  |   85 
 bootstrap.conf |   17 +++
 4 files changed, 153 insertions(+), 14 deletions(-)
 create mode 100644 README-prereq

diff --git a/README-hacking b/README-hacking
index 2e3c83a..f51bb01 100644
--- a/README-hacking
+++ b/README-hacking
@@ -8,8 +8,8 @@ These requirements do not apply when building from a distribution tarball.
 We've opted to keep only the highest-level sources in the GIT repository.
 This eases our maintenance burden, (fewer merges etc.), but imposes more
 requirements on anyone wishing to build from the just-checked-out sources.
-For example, you have to use the latest stable versions of the maintainer
-tools we depend upon, including:
+Specific tools and versions will be checked for and listed by the
+bootstrap script shown below, and will include:
 
 - Automake 
 - Autoconf 
@@ -22,13 +22,15 @@ tools we depend upon, including:
 - Rsync 
 - Tar 
 
-Valgrind  is also highly recommended, if
-Valgrind supports your architecture.
-
 Only building the initial full source tree will be a bit painful.
 Later, a plain `git pull && make' should be sufficient.
 
-* LZMA
+- Valgrind
+
+Valgrind  is also highly recommended, if
+Valgrind supports your architecture. See also README-valgrind.
+
+- LZMA
 
 This package's build procedure uses LZMA to create a compressed
 distribution tarball.  Using this feature of Automake requires
@@ -40,23 +42,24 @@ from .
 
 You can get a copy of the source repository like this:
 
-	$ git clone git://git.sv.gnu.org/coreutils
+$ git clone git://git.sv.gnu.org/coreutils
+$ cd coreutils
 
-The next step is to get other files needed to build, which are
-extracted from other source packages:
+The next step is to get and check other files needed to build,
+which are extracted from other source packages:
 
-	$ ./bootstrap
+$ ./bootstrap
 
 And there you are!  Just
 
-	$ ./configure
-	$ make
-	$ make check
+$ ./configure
+$ make
+$ make check
 
 At this point, there should be no difference between your local copy,
 and the GIT master copy:
 
-	$ git diff
+$ git diff
 
 should output no difference.
 
@@ -78,3 +81,7 @@ GNU General Public License for more details.
 
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see .
+
+Local Variables:
+indent-tabs-mode: nil
+End:
diff --git a/README-prereq b/README-prereq
new file mode 100644
index 000..7456114
--- /dev/null
+++ b/README-prereq
@@ -0,0 +1,30 @@
+Detailed below are concrete examples for
+getting the prerequisites for particular systems.
+
+- linux - fedora
+
+  This shows the steps for getting the required tools to build coreutils 7.0
+  on a Fedora 8 system. We try to use official packages where possible.
+  The 3 methods described for making these required packages available, should
+  help clarify build requirements on any linux system at least.
+
+  1. Make sure offical distro git package is installed
+# yum install git
+
+  2. The distro autoconf is too old, but there is a newer one available
+  so we rebuild that and make it available to the full system:
+# yum install emacs #autoconf build requires emac

Re: Patch to check for required programs when building from source checkout

2008-10-22 Thread Pádraig Brady
Eric Blake wrote:
> According to Pádraig Brady on 10/21/2008 4:04 PM:
>> echo "$ver" | sed '
>>   s/^\([0-9]\{,\}\)\.\([0-9]\{,\}\)[.0]*$/\1.\2.0/; #1.10  -> 
>> 1.10.0
>>   s/^\([0-9]\{,\}\)\.\([0-9]\{,\}\)\([a-z]\)/\1.\2.99\3/;   #1.10a -> 
>> 1.10.99a
> 
> For what it's worth, Autoconf does this by converting \([0-9]+\)\([a-z]+\)
> to \1+1, -1, radix36(\2).  In other words, 1.10a becomes 1.11.-1.10.
> 
> - From there, it is a simple numerical comparison, supplying a 0 for any
> implicit field, and declaring results at the left-most field that differs:
>  1.10.x < 1.11.-1.x, but 1.11.-1 < 1.11.0
> 
> Hmm.  Since bootstrap requires the existence of autoconf, why not just use
> autoconf, instead of reimplementing this in sed?
> 
> echo 'm4_divert(0)m4_version_compare('$ver,$prereq')' \
>   | autom4te --language=m4sugar -
> 
> outputs -1 if $ver is smaller, 0 if equal, 1 if greater than $prereq.

Cool tip, thanks!

There's a bit of a chicken and egg scenario here though :)
bootstrap does call autoconf but it doesn't explicitly depend on it.
Also if I use the above then I need to special case autoconf checking
which further complicates things. So leaving as is for now,
but I'll think more about it. If we want to be fully compatible
with old version format checking, then I agree we should not use sed,
and use something like:

#-1 if $ver1 < $ver2
# 0 if $ver1 = $ver2
# 1 if $ver1 > $ver2
#-2 on error
version_compare() {
  ver1="$1"
  ver2="$2"

  echo "m4_divert(0)m4_version_compare($ver1,$ver2)" |
  autom4te --language=m4sugar - 2>/dev/null || echo -2
}

latest version is attached
(minor tweaks compared to previous).

cheers,
Pádraig.

>From b9e5fe8076e7a55f152e5ffbd841310ba4994838 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[EMAIL PROTECTED]>
Date: Tue, 21 Oct 2008 22:40:12 +0100
Subject: [PATCH] Add better checks and docs for build tools

Prompted by a report from Ed Avis:
<http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/14710>

* README-hacking: Organise LZMA and Valgrind as
as optional requirements rather than in their own sections.
Mention bootstrap will now check tool versions.
* README-prereq: Make a start on specific instructions
for optaining build tools. Currently we just have notes
for Fedora linux.
* bootstrap.conf: Add the list of tools and versions required.
* bootstrap: Add the logic to check for the required tools,
and list all required tools and versions if any are missing.
---
 README-hacking |   35 +-
 README-prereq  |   30 +++
 bootstrap  |   86 
 bootstrap.conf |   17 +++
 4 files changed, 154 insertions(+), 14 deletions(-)
 create mode 100644 README-prereq

diff --git a/README-hacking b/README-hacking
index 2e3c83a..f51bb01 100644
--- a/README-hacking
+++ b/README-hacking
@@ -8,8 +8,8 @@ These requirements do not apply when building from a distribution tarball.
 We've opted to keep only the highest-level sources in the GIT repository.
 This eases our maintenance burden, (fewer merges etc.), but imposes more
 requirements on anyone wishing to build from the just-checked-out sources.
-For example, you have to use the latest stable versions of the maintainer
-tools we depend upon, including:
+Specific tools and versions will be checked for and listed by the
+bootstrap script shown below, and will include:
 
 - Automake <http://www.gnu.org/software/automake/>
 - Autoconf <http://www.gnu.org/software/autoconf/>
@@ -22,13 +22,15 @@ tools we depend upon, including:
 - Rsync <http://samba.anu.edu.au/rsync/>
 - Tar <http://www.gnu.org/software/tar/>
 
-Valgrind <http://valgrind.org/> is also highly recommended, if
-Valgrind supports your architecture.
-
 Only building the initial full source tree will be a bit painful.
 Later, a plain `git pull && make' should be sufficient.
 
-* LZMA
+- Valgrind
+
+Valgrind <http://valgrind.org/> is also highly recommended, if
+Valgrind supports your architecture. See also README-valgrind.
+
+- LZMA
 
 This package's build procedure uses LZMA to create a compressed
 distribution tarball.  Using this feature of Automake requires
@@ -40,23 +42,24 @@ from <http://tukaani.org/lzma/>.
 
 You can get a copy of the source repository like this:
 
-	$ git clone git://git.sv.gnu.org/coreutils
+$ git clone git://git.sv.gnu.org/coreutils
+$ cd coreutils
 
-The next step is to get other files needed to build, which are
-extracted from other source packages:
+The next step is to get and check other files needed to build,
+which are extracted from other source packages:
 
-	$ ./bootstrap
+$ ./bootstrap
 
 And there you are!  Just
 
-

Re: remove --bignum from 'factor'

2008-10-24 Thread Pádraig Brady
Paul Eggert wrote:
> Here's a patch to remove the --bignum and --no-bignum options from
> 'factor'.  The case for removing --bignum isn't as strong as that for
> 'expr', but still, it seems to me that these options are not needed and
> complicate the documentation unnecessarily.
> 
> 
> Remove --bignum and --no-bignum from 'factor'.

+1

In general there should be a very strong case
required for adding new options.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


  1   2   3   4   5   6   7   8   9   10   >