* Pentecost weekend pull request
@ 2011-06-13 15:13 Sami Kerola
2011-06-20 10:19 ` Karel Zak
0 siblings, 1 reply; 4+ messages in thread
From: Sami Kerola @ 2011-06-13 15:13 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 6423 bytes --]
Hi Karel et.al.
I feel a bit embarrassed to see how many patches I wrote. My
excuse is long, mostly rainy, weekend. Here comes a pull request;
[01/33] mount: take xalloc.h in use
Basiclly code reuse.
[02/33] checktty: NGROUP -> sysconf
This patch was sent earlier, see details bellow.
date Thu, Jun 9, 2011 at 21:31
subject Re: [PATCH 3/6] checktty: Use NGROUPS_MAX instead of NGROUPS
[03/33] checktty: fix unused parameters
Ditto.
[04/33] login-utils: include fix
Inspired what Pádraig Brady did with coreutils I thought
to give a shot to include-what-you-use as well.
http://lists.gnu.org/archive/html/coreutils/2011-06/msg00004.html
The utility clearly works. So it is a good time to think
if checking headers in all files should be TODO item?
[05/33] namei: add --version option
[06/33] namei: use xalloc.h
[07/33] namei: fix to argument handling
The namei patches are trivial.
[08/33] mcookie: use warnx, long options and help screen
[09/33] docs: inform about mcookie long options
[10/33] mcookie: change coding style
The mcookie patches are trivial, with exception of docs
change. Updating manual pages with help2man seems to be
quite good idea. Using the output as is is pretty brain
dead idea, but copy pasting bits of syntax etc seems to
work great. Is this an approach you would like to see in
future?
[11/33] whereis: maintenance fixes
[12/33] cal: maintenance fixes
[13/33] docs: inform about cal long options
Trivial changes.
[14/33] rename: verbose option & maintenance fixes
[15/33] docs: rename.1 verbose, long options and warning
Looking the code this utility seems to be in wrong
package. Rewriting the command to rely heavily on gnulib
copy.h and backupfile.h would make this to be good
addition to coreutils, assuming they accept the command.
Or it could time to deprecate the whole command, and
recommend users to multimove by using scripts.
[16/33] misc-utils: remove misleading README files
[17/33] build-sys: move write to term-utils directory
Maintainer style activities.
[18/33] write: maintenance fixes
[19/33] wipefs: add version printing & compiler warning
[20/33] uuidgen: add long options
[21/33] docs: uuidgen.1 mention long options
[22/33] uuid: define UUID_STR_LEN in uuid.h
[23/33] uuidd: maintenance fixes
[24/33] docs: mention long options in uuidd.8 manual page
[25/33] look: add long options
[26/33] docs: look.1 manual rewrote
Trivial changes.
[27/33] getopt: use xalloc.h
Resubmission of getopts maintenance, see
http://permalink.gmane.org/gmane.linux.utilities.util-linux-ng/3822
[28/33] getopt: remove unnecessary free()
Trivial change.
[29/33] getopt: make user getopt_long parsing to use function pointer
Without this the 33/33 would look unreadable at the point
where the function pointer is used.
[30/33] build-sys: remove unnecessary files from getopt
[31/33] getopt: options struct, usage and version outputs
[32/33] getopt: inform where to send bug reports
I wonder should the config.h PACKAGE_BUGREPORT be
util-linux mail list...
[33/33] getopt: fix coding style
Trivial change.
---
The following changes since commit 42fe2b2b2d19d09241bd7c3dd811463e0095d4aa:
Merge branch 'master', remote-tracking branch 'upstream/master' (2011-06-11 10:10:01 +0200)
are available in the git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git wknd23
disk-utils/mkswap.c | 2 +-
getopt/Makefile.am | 3 +-
getopt/getopt-1.1.3.lsm | 16 --
getopt/getopt-test.bash | 6 -
getopt/getopt-test.tcsh | 7 -
getopt/getopt.c | 514 ++++++++++++++++++------------------
include/xalloc.h | 23 ++
libuuid/src/tst_uuid.c | 2 +-
libuuid/src/uuid.h | 3 +
login-utils/checktty.c | 32 ++-
login-utils/islocal.c | 2 +-
misc-utils/.gitignore | 1 -
misc-utils/Makefile.am | 21 +--
misc-utils/README.flushb | 5 -
misc-utils/README.namei2 | 14 -
misc-utils/README.reset | 20 --
misc-utils/cal.1 | 94 ++++----
misc-utils/cal.c | 166 +++++++-----
misc-utils/look.1 | 121 ++++-----
misc-utils/look.c | 51 +++-
misc-utils/mcookie.1 | 27 ++-
misc-utils/mcookie.c | 267 +++++++++++--------
misc-utils/namei.1 | 4 +
misc-utils/namei.c | 47 ++--
misc-utils/rename.1 | 36 ++-
misc-utils/rename.c | 93 +++++---
misc-utils/uuidd.8 | 88 +++----
misc-utils/uuidd.c | 114 +++++---
misc-utils/uuidgen.1 | 14 +-
misc-utils/uuidgen.c | 38 +++-
misc-utils/whereis.c | 141 +++++++----
misc-utils/wipefs.c | 13 +-
mount/Makefile.am | 2 +-
mount/devname.c | 3 +-
mount/fstab.c | 4 +-
mount/lomount.c | 4 +-
mount/mount.c | 4 +-
mount/mount_mntent.c | 3 +-
mount/sundries.c | 18 +--
mount/sundries.h | 7 +-
mount/umount.c | 3 +-
mount/xmalloc.c | 48 ----
mount/xmalloc.h | 14 -
term-utils/.gitignore | 1 +
term-utils/Makefile.am | 14 +-
{misc-utils => term-utils}/write.1 | 0
{misc-utils => term-utils}/write.c | 215 ++++++++-------
47 files changed, 1236 insertions(+), 1089 deletions(-)
delete mode 100644 getopt/getopt-1.1.3.lsm
delete mode 100755 getopt/getopt-test.bash
delete mode 100755 getopt/getopt-test.tcsh
delete mode 100644 misc-utils/README.flushb
delete mode 100644 misc-utils/README.namei2
delete mode 100644 misc-utils/README.reset
delete mode 100644 mount/xmalloc.c
delete mode 100644 mount/xmalloc.h
rename {misc-utils => term-utils}/write.1 (100%)
rename {misc-utils => term-utils}/write.c (67%)
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Pentecost weekend pull request
2011-06-13 15:13 Pentecost weekend pull request Sami Kerola
@ 2011-06-20 10:19 ` Karel Zak
2011-06-25 15:26 ` Sami Kerola
0 siblings, 1 reply; 4+ messages in thread
From: Karel Zak @ 2011-06-20 10:19 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Mon, Jun 13, 2011 at 05:13:50PM +0200, Sami Kerola wrote:
> I feel a bit embarrassed to see how many patches I wrote. My
> excuse is long, mostly rainy, weekend. Here comes a pull request;
:-)
> [01/33] mount: take xalloc.h in use
It would be better to keep my_free() in mount/ sources only. I don't
think that we will use this crap outside the mount code.
Note that the final solution will be libmount/mount.c as the mount(8)
command implementation. So, let's keep the current mount/* code in
maintenance mode and don't try to rewrite it :-)
> [02/33] checktty: NGROUP -> sysconf
> This patch was sent earlier, see details bellow.
>
> date Thu, Jun 9, 2011 at 21:31
> subject Re: [PATCH 3/6] checktty: Use NGROUPS_MAX instead of NGROUPS
You cannot use xalloc() (and friends) in login(1). The login has to
call pam_end and report to syslog before exit.
I'd like to clean up login(1), it means:
- code refactoring
- support PAM or /etc/{passwd,shadow} *only*
- remove checktty support
- remove cryptocard support
- remove kerberos support
> [04/33] login-utils: include fix
> Inspired what P?draig Brady did with coreutils I thought
> to give a shot to include-what-you-use as well.
> http://lists.gnu.org/archive/html/coreutils/2011-06/msg00004.html
>
> The utility clearly works. So it is a good time to think
> if checking headers in all files should be TODO item?
Nice!
What about to add any wrapper to the tools/ directory and call it
from our top-level Makefile?
We already have some stuff there, and I use it before release.
> [10/33] mcookie: change coding style
> The mcookie patches are trivial, with exception of docs
> change. Updating manual pages with help2man seems to be
> quite good idea. Using the output as is is pretty brain
> dead idea, but copy pasting bits of syntax etc seems to
> work great. Is this an approach you would like to see in
> future?
It depends, the --help output is usually very brief and space-saving.
Don't forget that GNU guys hate man pages and all their documentation
is in the horrible info files, so help2man makes sense for them ;-)
We don't have info files, but we have real man pages.
> [11/33] whereis: maintenance fixes
I can imagine more than one patch for these changes.
> [12/33] cal: maintenance fixes
I'll use this patch, but next time don't mix any real changes in the
code with indentation changes. Please!
> [15/33] docs: rename.1 verbose, long options and warning
> Looking the code this utility seems to be in wrong
> package. Rewriting the command to rely heavily on gnulib
> copy.h and backupfile.h would make this to be good
> addition to coreutils, assuming they accept the command.
Yep, go ahead and ask at coreutils mailing list.
> Or it could time to deprecate the whole command, and
> recommend users to multimove by using scripts.
The command is used by many users. I don't think that deprecation is
a good idea. And I also don't think that change the default behavior
is a good idea. Maybe we can add --dry-run.
> [18/33] write: maintenance fixes
Don't use
return (1);
use
return 1;
The "return" is not a function.
> [23/33] uuidd: maintenance fixes
- remove die() -- use err()
- remove printf() + exit() -- use err[x]()
- print all error messages (!= debug) to stderr
> [32/33] getopt: inform where to send bug reports
> I wonder should the config.h PACKAGE_BUGREPORT be
> util-linux mail list...
Probably ;-)
We shouldn't use PACKAGE_BUGREPORT in man pages or --help output. I
don't think that upstream is the right place for end-users and their
bug reports.
The ideal solution is downstream --patch--> upstream ;-)
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Pentecost weekend pull request
2011-06-20 10:19 ` Karel Zak
@ 2011-06-25 15:26 ` Sami Kerola
2011-06-27 15:29 ` Karel Zak
0 siblings, 1 reply; 4+ messages in thread
From: Sami Kerola @ 2011-06-25 15:26 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On Mon, Jun 20, 2011 at 12:19, Karel Zak <kzak@redhat.com> wrote:
> On Mon, Jun 13, 2011 at 05:13:50PM +0200, Sami Kerola wrote:
>
>> [01/33] mount: take xalloc.h in use
>
> It would be better to keep my_free() in mount/ sources only. I don't
> think that we will use this crap outside the mount code.
>
> Note that the final solution will be libmount/mount.c as the mount(8)
> command implementation. So, let's keep the current mount/* code in
> maintenance mode and don't try to rewrite it :-)
The original patch is replaced with a above note added to README.
>> [02/33] checktty: NGROUP -> sysconf
>> This patch was sent earlier, see details bellow.
>>
>> date Thu, Jun 9, 2011 at 21:31
>> subject Re: [PATCH 3/6] checktty: Use NGROUPS_MAX instead of NGROUPS
>
> You cannot use xalloc() (and friends) in login(1). The login has to
> call pam_end and report to syslog before exit.
>
> I'd like to clean up login(1), it means:
>
> - code refactoring
> - support PAM or /etc/{passwd,shadow} *only*
> - remove checktty support
> - remove cryptocard support
> - remove kerberos support
This patch is gone. IMHO the above should be in TODO file.
>> [04/33] login-utils: include fix
>> Inspired what P?draig Brady did with coreutils I thought
>> to give a shot to include-what-you-use as well.
>> http://lists.gnu.org/archive/html/coreutils/2011-06/msg00004.html
>>
>> The utility clearly works. So it is a good time to think
>> if checking headers in all files should be TODO item?
>
> Nice!
>
> What about to add any wrapper to the tools/ directory and call it
> from our top-level Makefile?
>
> We already have some stuff there, and I use it before release.
Funny enough in the coreutils list same question was asked pretty
much immediately. I agree it would be awesome to get automatic
header checking, but for now that is not easy to achieve.
The utility relies on llvm source tree, and it gives lots of
false positive indications. For instance iwyu proposes nls.h
should be removed and replaced with libintl.h. Same happens with
c.h and err.h.
For now I recommend to use this utility when one is working with
some particular file. Something like `if you are going to touch
the file check the headers as well'.
>> [10/33] mcookie: change coding style
>> The mcookie patches are trivial, with exception of docs
>> change. Updating manual pages with help2man seems to be
>> quite good idea. Using the output as is is pretty brain
>> dead idea, but copy pasting bits of syntax etc seems to
>> work great. Is this an approach you would like to see in
>> future?
>
> It depends, the --help output is usually very brief and space-saving.
>
> Don't forget that GNU guys hate man pages and all their documentation
> is in the horrible info files, so help2man makes sense for them ;-)
> We don't have info files, but we have real man pages.
I did not mean help2man output should be used as is. Taking for
instance .TH line or option \fB\-h\fR, \fB\-\-help\fR lines
sounds reasonable for me. I let the patch be what it was.
>> [11/33] whereis: maintenance fixes
>
> I can imagine more than one patch for these changes.
Split to 4 patches.
>> [12/33] cal: maintenance fixes
>
> I'll use this patch, but next time don't mix any real changes in the
> code with indentation changes. Please!
Sorry. Split to 3 patches.
>> [15/33] docs: rename.1 verbose, long options and warning
>> Looking the code this utility seems to be in wrong
>> package. Rewriting the command to rely heavily on gnulib
>> copy.h and backupfile.h would make this to be good
>> addition to coreutils, assuming they accept the command.
>
> Yep, go ahead and ask at coreutils mailing list.
Eventually yes.
>> Or it could time to deprecate the whole command, and
>> recommend users to multimove by using scripts.
>
> The command is used by many users. I don't think that deprecation is
> a good idea. And I also don't think that change the default behavior
> is a good idea. Maybe we can add --dry-run.
This is exactly why merging to coreutils might work. Providing
the same switches as there is for mv or cp is fairly easy with
gnulib. IMHO the tool is a little too unixy in doing exactly what
user asks how ever stupid it might be. Having --backup,
--interactive and --no-globber just sound right to me.
I am not sure is --dry-run in this case good idea. There's no
guarantees contents on file system have not changed in between
dry and next run. For example the command is not reentant safe.
User might simply get over confident the dry run did not report
anything and end up loosing data.
>> [18/33] write: maintenance fixes
>
> Don't use
>
> return (1);
>
> use
> return 1;
>
> The "return" is not a function.
Fixed.
>> [23/33] uuidd: maintenance fixes
>
> - remove die() -- use err()
> - remove printf() + exit() -- use err[x]()
> - print all error messages (!= debug) to stderr
Done and split to multiple patches.
>> [32/33] getopt: inform where to send bug reports
>> I wonder should the config.h PACKAGE_BUGREPORT be
>> util-linux mail list...
>
> Probably ;-)
>
> We shouldn't use PACKAGE_BUGREPORT in man pages or --help output. I
> don't think that upstream is the right place for end-users and their
> bug reports.
>
> The ideal solution is downstream --patch--> upstream ;-)
This patch is gone.
All the patches are available in the git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git wknd23
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Pentecost weekend pull request
2011-06-25 15:26 ` Sami Kerola
@ 2011-06-27 15:29 ` Karel Zak
0 siblings, 0 replies; 4+ messages in thread
From: Karel Zak @ 2011-06-27 15:29 UTC (permalink / raw)
To: kerolasa; +Cc: util-linux
On Sat, Jun 25, 2011 at 05:26:35PM +0200, Sami Kerola wrote:
> All the patches are available in the git repository at:
> git://github.com/kerolasa/lelux-utiliteetit.git wknd23
Applied, thanks!
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-06-27 15:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-13 15:13 Pentecost weekend pull request Sami Kerola
2011-06-20 10:19 ` Karel Zak
2011-06-25 15:26 ` Sami Kerola
2011-06-27 15:29 ` Karel Zak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox