Re: [PATCH] ldattach(8)

[Tilman Schmidt <tilman@xxxxxxx>]

Karel,

thanks for your comments. I'll address most of them with the next version,
but have some specific replies:

Am 06.02.2008 01:27 schrieb Karel Zak:
> > +#ifndef N_GIGASET_M101
> > +#define N_GIGASET_M101 16
> > +#endif
>
>  What about fallbacks for the others line disciplines (I assume there
>  is dependence on kernel version)?

N_GIGASET_M101 in v2.6.21 and, as I just noticed, N_SLCAN in upcoming
v2.6.25 are the only recent additions. Apart from these, the list of
line disciplines hasn't changed since kernel development moved to git.
Also I have omitted line disciplines which have an N_ code defined
but no implementation.

> > +    static const struct option opttbl[] = {
> > +	{"speed", 1, 0, 's'},
> > +	{"debug", 0, 0, 'd'},
> > +	{0, 0, 0, 0}
> > +    };
> > +
> > +    /* parse options */
> > +    while ((optc = getopt_long(argc, argv, "d78neo12s:", opttbl, NULL)) >= 
>
>  I don't see --help and --version. Note, as a --version output use
>
>     printf(_("ldattach from %s\n"), PACKAGE_STRING);

Ah, I didn't know those were mandatory. Will add. --help will have
no different effect than calling the program without any arguments,
though. Hope that's ok.

>  I also don't see long options for -e, -o and -n.

I don't think they'll be useful but for the sake of completeness
I'll add long options for all the short options.

> > > > > +static void usage(const char *prog)
> > > > > +{
> > > > > +    fprintf(stderr,
> > > > > +	    "Usage: %s [ -d78neo12 ] [ -s <speed> ] <ldisc> <device>\n", prog);
> > > > > +    exit(EXIT_FAILURE);
> > > > > +}
> > > 
> > > i feel like "prog" should be a program-level static rather than passing around 
>
>  glibc  program_invocation_short_name  ? :-)

Sorry, I don't get the joke. Is that a serious suggestion?

> > > > > +	fprintf(stderr, "%s: cannot get terminal attributes for %s: %s\n",
> > > > > +		prog, dev, strerror(errno));
> > > > > +	exit(EXIT_FAILURE);
> > > > > ...
> > > > > <many more>
> > > 
> > > could do with writing an error function and/or macro to make things cleaner
>
>  #include <err.h> ... err()  -- very luxury function:

Very nice. Thanks for pointing those out to me.

Thanks
Tilman

--
Tilman Schmidt                          E-Mail: tilman@xxxxxxx
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)

Attachment: signature.asc
Description: OpenPGP digital signature