Date: Sat, 2 Mar 2002 08:25:10 +0000 From: "J. Mallett" <jmallett@FreeBSD.ORG> To: "Tim J. Robbins" <tim@robbins.dropbear.id.au> Cc: "J. Mallett" <jmallett@FreeBSD.ORG>, freebsd-standards@FreeBSD.ORG Subject: Re: base64 support for uuencode/uudecode Message-ID: <20020302082509.A16290@FreeBSD.ORG> In-Reply-To: <20020302135621.A96061@descent.robbins.dropbear.id.au> References: <20020302010401.A18553@FreeBSD.ORG> <20020302135621.A96061@descent.robbins.dropbear.id.au>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Mar 02, 2002 at 01:56:21PM +1100, Tim J. Robbins wrote:
> On Sat, Mar 02, 2002 at 01:04:02AM +0000, J. Mallett wrote:
>
> (in uuencode.c)
>
> > +FILE *output = stdout;
>
> > + outfile = NULL;
> > +
> > + while ((ch = getopt(argc, argv, "mo:")) != -1) {
> > + switch (ch) {
>
> > + case 'o':
> > + outfile = optarg;
> > + break;
> > +
> > + if (outfile != NULL) {
> > + output = fopen(outfile, "w+");
> > + if (output == NULL)
> > + err(1, "Unable to open %s for output!\n", outfile);
> > + chmod(outfile, 0644);
> > + }
>
> freopen(outfile, "w", stdout)
> would be a cleaner way to approach this. The chmod() seems redundant here;
> with the default umask, 644 will be used anyway. You could, however, chmod
> 600 the file here to protect it while it's being decoded and before the
> original mode is restored.
Sorry, I took the chmod when mindlessly copying from uudecode.
> > if (ferror(stdout))
> > errx(1, "write error");
>
> I think this should be ferror(outfile) if you don't want to freopen().
Yeah, thanks.
> > + fprintf(output, "begin-base64 %o %s\n", mode, *av);
> > + while ((n = fread(buf, sizeof(buf[0]),
> > + (sizeof(buf) / sizeof(buf[0])), stdin))) {
>
> This can be tidied up by knowing that sizeof(char) == 1:
> fread(buf, 1, sizeof(buf), stdin);
You're actually the one who got me used to doing that, heh!
But anyway, thanks, fixed.
> (in uudecode.c)
>
> > - } while (strncmp(buf, "begin ", 6) ||
> > - fnmatch("begin [0-7]* *", buf, 0));
> > + } while (strncmp(buf, "begin", 5) != 0);
> > +
> > + if (strncmp(buf, "begin-base64", 12) == 0)
> > + base64 = 1;
> >
> > if (oflag) {
> > - (void)sscanf(buf, "begin %o ", &mode);
> > + if (base64)
> > + (void)sscanf(buf, "begin-base64 %o ", &mode);
> > + else
> > + (void)sscanf(buf, "begin %o ", &mode);
> > if (strlcpy(buf, outfile, sizeof(buf)) >= sizeof(buf)) {
> > warnx("%s: filename too long", outfile);
> > return (1);
> > }
> > - } else
> > - (void)sscanf(buf, "begin %o %[^\n\r]", &mode, buf);
> > + } else {
> > + if (base64)
> > + (void)sscanf(buf, "begin-base64 %o %[^\n\r]", &mode, buf);
> > + else
> > + (void)sscanf(buf, "begin %o %[^\n\r]", &mode, buf);
> > + }
>
> This code is messy, but was already messy long before you touched it.
>
> Also, P1003.1-2001 requires that uudecode must be able to decode the result
> of uuencode. uuencode may encode the file's mode in octal or symbolically.
> This means that the scanf()'s with %o are incorrect, and that uudecode should
> use getmode()/setmode() (like chmod(1) does) instead. I think it'd be cleaner
> to avoid the use of scanf() here altogether.
>
> For example, the following (apparently) should be accepted:
> begin u=rw,go=r foo.txt
> as well as:
> begin u+x foo.txt
> which uses modes relative to an already existing foo.txt file.
For similarity I haven't touched this yet,... I'd like to get base64 in
and then concentrate on fixing historically broken parts.
> In general though, I like the way you've used library functions instead of
> writing the encoding/decoding code yourself.
I don't like to duplicate library code, and decoding and encoding base64
should be library code...
Eventually we should repo copy to libutil, and possibly move away from the
networking namespace, for general purpose use of this, and put protos into
<libutil.h>, but for now, this is the most approachable way of doing it.
Plus it keeps code bloat down, in general.
> Tim
Always a pleasure.
/j.
An aside: Am I grouping the output chunks acceptably?
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-standards" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020302082509.A16290>