From owner-freebsd-standards Fri Mar 1 19: 9:16 2002 Delivered-To: freebsd-standards@freebsd.org Received: from descent.robbins.dropbear.id.au (068.c.001.mel.iprimus.net.au [203.134.131.68]) by hub.freebsd.org (Postfix) with ESMTP id 9CC2237B400; Fri, 1 Mar 2002 19:09:01 -0800 (PST) Received: (from tim@localhost) by descent.robbins.dropbear.id.au (8.11.6/8.11.6) id g222uNg96111; Sat, 2 Mar 2002 13:56:23 +1100 (EST) (envelope-from tim) Date: Sat, 2 Mar 2002 13:56:21 +1100 From: "Tim J. Robbins" To: "J. Mallett" Cc: freebsd-standards@FreeBSD.ORG Subject: Re: base64 support for uuencode/uudecode Message-ID: <20020302135621.A96061@descent.robbins.dropbear.id.au> References: <20020302010401.A18553@FreeBSD.ORG> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <20020302010401.A18553@FreeBSD.ORG>; from jmallett@FreeBSD.ORG on Sat, Mar 02, 2002 at 01:04:02AM +0000 Sender: owner-freebsd-standards@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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