Re: midicat(1): use err(3)
On Wed, Nov 30, 2022 at 09:20:26AM -0600, Scott Cheloha wrote:
> Couple related things:
>
> - Use err(3) everywhere.
>
> For many of these errors we are not currently printing the errno
> string. Is there any reason not to do so? The errno string is
> useful.
>
> - Set ifile/ofile to "stdin"/"stdout" if the user passes in
> "-" to make the err(3) message a little more obvious.
>
> - Add a usage() function.
>
> ok?
>
ok except for changes below
> @@ -126,20 +114,16 @@ main(int argc, char **argv)
> else
> mode = MIO_IN;
> ih = mio_open(port0, mode, 0);
> - if (ih == NULL) {
> - fprintf(stderr, "%s: couldn't open port\n", port0);
> - return 1;
> - }
> + if (ih == NULL)
> + err(1, "%s: couldn't open port", port0);
>
> /* open second port, output only */
> if (port1 == NULL)
> oh = ih;
> else {
> oh = mio_open(port1, MIO_OUT, 0);
> - if (oh == NULL) {
> - fprintf(stderr, "%s: couldn't open port\n", port1);
> - exit(1);
> - }
> + if (oh == NULL)
> + err(1, "%s: couldn't open port", port1);
The mio_xxx functions don't set errno, so you should use errx() and
warnx() here
> @@ -152,26 +136,26 @@ main(int argc, char **argv)
> if (len == 0)
> break;
> if (len == -1) {
> - perror("stdin");
> + warn("%s", ifile);
> break;
> }
> } else {
> len = mio_read(ih, buf, sizeof(buf));
> if (len == 0) {
> - fprintf(stderr, "%s: disconnected\n", port0);
> + warn("%s: disconnected", port0);
> break;
and here
> } else {
> n = mio_write(oh, buf, len);
> if (n != len) {
> - fprintf(stderr, "%s: disconnected\n", port1);
> + warn("%s: disconnected", port1);
> break;
and here
Re: midicat(1): use err(3)
On Wed, Nov 30, 2022 at 08:55:45AM -0700, Todd C. Miller wrote: > On Wed, 30 Nov 2022 09:20:26 -0600, Scott Cheloha wrote: > > > Couple related things: > > > > - Use err(3) everywhere. > > > > For many of these errors we are not currently printing the errno > > string. Is there any reason not to do so? The errno string is > > useful. > > OK > > > - Set ifile/ofile to "stdin"/"stdout" if the user passes in > > "-" to make the err(3) message a little more obvious. > > OK > > > - Add a usage() function. > > usage() should probably be marked __dead in the prototype. Agreed. While here, can you sync usage and manual? I think the latter is fine and the former should be just that: $ man -h midicat midicat [-d] [-i file] [-o file] [-q port] $ midicat -h midicat: unknown option -- h usage: midicat [-d] [-i in-file] [-o out-file] [-q in-port] [-q out-port] > > Otherwise OK millert@ > > - todd >
Re: midicat(1): use err(3)
On Wed, 30 Nov 2022 09:20:26 -0600, Scott Cheloha wrote: > Couple related things: > > - Use err(3) everywhere. > > For many of these errors we are not currently printing the errno > string. Is there any reason not to do so? The errno string is > useful. OK > - Set ifile/ofile to "stdin"/"stdout" if the user passes in > "-" to make the err(3) message a little more obvious. OK > - Add a usage() function. usage() should probably be marked __dead in the prototype. Otherwise OK millert@ - todd
midicat(1): use err(3)
Couple related things:
- Use err(3) everywhere.
For many of these errors we are not currently printing the errno
string. Is there any reason not to do so? The errno string is
useful.
- Set ifile/ofile to "stdin"/"stdout" if the user passes in
"-" to make the err(3) message a little more obvious.
- Add a usage() function.
ok?
Index: midicat.c
===
RCS file: /cvs/src/usr.bin/midicat/midicat.c,v
retrieving revision 1.4
diff -u -p -r1.4 midicat.c
--- midicat.c 30 Nov 2022 14:56:45 - 1.4
+++ midicat.c 30 Nov 2022 15:13:01 -
@@ -22,8 +22,7 @@
#include
#include
-char usagestr[] = "usage: midicat [-d] [-i in-file] [-o out-file] "
- "[-q in-port] [-q out-port]\n";
+void usage(void);
int
main(int argc, char **argv)
@@ -50,10 +49,8 @@ main(int argc, char **argv)
port0 = optarg;
else if (port1 == NULL)
port1 = optarg;
- else {
- fputs("too many -q options\n", stderr);
- return 1;
- }
+ else
+ errx(1, "too many -q options");
break;
case 'i':
ifile = optarg;
@@ -62,32 +59,25 @@ main(int argc, char **argv)
ofile = optarg;
break;
default:
- goto bad_usage;
+ usage();
}
}
argc -= optind;
argv += optind;
- if (argc != 0) {
- bad_usage:
- fputs(usagestr, stderr);
- return 1;
- }
+ if (argc != 0)
+ usage();
/* we don't support more than one data flow */
- if (ifile != NULL && ofile != NULL) {
- fputs("-i and -o are exclusive\n", stderr);
- return 1;
- }
+ if (ifile != NULL && ofile != NULL)
+ errx(1, "-i and -o are exclusive");
/* second port makes sense only for port-to-port transfers */
- if (port1 != NULL && !(ifile == NULL && ofile == NULL)) {
- fputs("too many -q options\n", stderr);
- return 1;
- }
+ if (port1 != NULL && !(ifile == NULL && ofile == NULL))
+ errx(1, "too many -q options");
/* if there're neither files nor ports, then we've nothing to do */
if (port0 == NULL && ifile == NULL && ofile == NULL)
- goto bad_usage;
+ usage();
/* if no port specified, use default one */
if (port0 == NULL)
@@ -95,24 +85,22 @@ main(int argc, char **argv)
/* open input or output file (if any) */
if (ifile) {
- if (strcmp(ifile, "-") == 0)
+ if (strcmp(ifile, "-") == 0) {
+ ifile = "stdin";
ifd = STDIN_FILENO;
- else {
+ } else {
ifd = open(ifile, O_RDONLY);
- if (ifd == -1) {
- perror(ifile);
- return 1;
- }
+ if (ifd == -1)
+ err(1, "%s", ifile);
}
} else if (ofile) {
- if (strcmp(ofile, "-") == 0)
+ if (strcmp(ofile, "-") == 0) {
+ ofile = "stdout";
ofd = STDOUT_FILENO;
- else {
+ } else {
ofd = open(ofile, O_WRONLY | O_CREAT | O_TRUNC, 0666);
- if (ofd == -1) {
- perror(ofile);
- return 1;
- }
+ if (ofd == -1)
+ err(1, "%s", ofile);
}
}
@@ -126,20 +114,16 @@ main(int argc, char **argv)
else
mode = MIO_IN;
ih = mio_open(port0, mode, 0);
- if (ih == NULL) {
- fprintf(stderr, "%s: couldn't open port\n", port0);
- return 1;
- }
+ if (ih == NULL)
+ err(1, "%s: couldn't open port", port0);
/* open second port, output only */
if (port1 == NULL)
oh = ih;
else {
oh = mio_open(port1, MIO_OUT, 0);
- if (oh == NULL) {
- fprintf(stderr, "%s: couldn't open port\n", port1);
- exit(1);
- }
+ if (oh == NULL)
+ err(1, "%s: couldn't open port", port1);
}
if (pledge("stdio", NULL) == -1)
@@ -152,26 +136,26 @@ main(int argc, char **argv)
if (len == 0)
break;
