On Tue, Apr 26, 2011 at 01:31:33PM +0200, Karel Zak wrote:
> On Mon, Apr 25, 2011 at 03:58:45PM +0200, Sami Kerola wrote:
> > from Karel Zak <kzak@xxxxxxxxxx>
> > date Thu, Apr 14, 2011 at 13:58
> > subject Re: [PATCH 1/8] agetty: use xalloc.h
> >
> > I have removed tailing whitespace and fixed identation (by '='
> > vim command ...), but it's still not enough. It would be nice to
> > clean up the code according to kernel Documentation/CodingStyle.
> > -- snip
> >
> > The scubbing consists;
> >
> > o Indentation changes.
> > o Removal of void casting.
> > o Removal of braces from single statements.
> > o Unified format for comments.
> > o Comparison with W. Venema's code from 1993 to clean few comment
> > mysteries.
> > o Old school introduction back to the top of the source file.
> > o Clean up of comments and/or marking them up with FIXME where
> > attention is needed.
> > o Web referrals added to further explanation to near the code or
> > comments which some could find difficult to understand why.
>
> Cool!
>
> > +/* Displayed before the login prompt. */
> > #ifdef SYSV_STYLE
> > -#define ISSUE "/etc/issue" /* displayed before the login prompt */
>
> It would be better to move this path to include/pathnames.h
>
> > #define P_(s) s
> > [...]
> > +P_((struct options * op, struct termios * tp, struct chardata * cp));
>
> Please, remove the P_() thing and use "static" for the functions.
>
> > int main(argc, argv)
> > - int argc;
> > - char **argv;
> > +int argc;
> > +char **argv;
>
> ... in 21st century:
>
> int main(int argc, char **argv);
>
> > case 'I':
> > - {
> > - char ch, *p, *q;
> > - int i;
> > -
> > - op->initstring = xmalloc(strlen(optarg) + 1);
> > -
> > - /* copy optarg into op->initstring decoding \ddd octal
> > - * codes into chars
> > - */
> > - q = op->initstring;
> > - p = optarg;
> > - while (*p) {
> > - if (*p == '\\') { /* know \\ means \ */
> > - p++;
> > + {
> > + char ch, *p, *q;
> > + int i;
> > +
> > + op->initstring = xmalloc(strlen(optarg) + 1);
> > +
> > + /*
> > + * Copy optarg into op->initstring decoding \ddd octal
> > + * codes into chars.
> > + */
>
> It would be better to use a separate function for this task. It's almost
> always better when the argv[] parsing (the switch() block) in main() is
> simple and readable. Please, use a separate patch for this change.
Hi Karel, Hi Sami,
I'd like to port my changes of agetty to support the mingetty
features to the current agetty version. Currently the patches
done by Sami are missed in the git HEAD.
My aim is a better support of the virtual console to be able
to have only one getty around in systemd. As my patches are
large and based on util-linux-2.19 I'd prefer the git HEAD
a bit more freezen :)
Werner
--
"Having a smoking section in a restaurant is like having
a peeing section in a swimming pool." -- Edward Burr
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
