Date: Sat, 2 Mar 2002 13:56:21 +1100 From: "Tim J. Robbins" <tim@robbins.dropbear.id.au> To: "J. Mallett" <jmallett@FreeBSD.ORG> Cc: freebsd-standards@FreeBSD.ORG Subject: Re: base64 support for uuencode/uudecode Message-ID: <20020302135621.A96061@descent.robbins.dropbear.id.au> In-Reply-To: <20020302010401.A18553@FreeBSD.ORG>; from jmallett@FreeBSD.ORG on Sat, Mar 02, 2002 at 01:04:02AM %2B0000 References: <20020302010401.A18553@FreeBSD.ORG>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
> if (ferror(stdout))
> errx(1, "write error");
I think this should be ferror(outfile) if you don't want to freopen().
> + 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);
(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.
In general though, I like the way you've used library functions instead of
writing the encoding/decoding code yourself.
Tim
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?20020302135621.A96061>