Hi,
it would be nice to have a regression test (see tests/) before you
start to rewrite any tool.
On Wed, Jul 28, 2010 at 06:32:32PM -0400, Davidlohr Bueso wrote:
> +/* if not global, we cannot free it when SIGINTs occur */
> +wchar_t *p;
horrible name for a global variable :-)
>
> void
> usage(void)
> @@ -143,3 +69,105 @@ usage(void)
> (void)fprintf(stderr, _("usage: rev [file ...]\n"));
> exit(1);
> }
Please, try to use usage() from (for example) sys-utils/fallocate.c.
I think that "rev -?" should not print the output to stderr.
> +static void
> +sig_handler(int signo)
> +{
> + switch(signo) {
> + case SIGINT: /* very used Ctrl-C when no file is passed as argv */
> + case SIGTERM:
> + if(p)
> + free(p);
please, don't use "if" before free.
> + }
> +
> + exit (EXIT_SUCCESS);
> +}
> +
> +int
> +main(int argc, char *argv[])
> +{
> + register char *filename;
> + register wchar_t *t;
I don't think that "register" is necessary here.
> + size_t len, buflen = 512;
what about BUFSIZ instead 512 bytes?
> + FILE *fp;
> + int ch, rval;
> +
> + setlocale(LC_ALL, "");
> + bindtextdomain(PACKAGE, LOCALEDIR);
> + textdomain(PACKAGE);
> +
> + signal(SIGINT, sig_handler);
> + signal(SIGTERM, sig_handler);
> +
> + /* if no mem left, no bother continuing */
> + p = malloc(buflen*sizeof(wchar_t));
do we really need to allocate the buffer before we parse argv[] ?
> + if (p == NULL)
> + err(1, _("unable to allocate bufferspace"));
err(EXIT_FAILURE, ....)
> + while ((ch = getopt(argc, argv, "")) != -1)
> + switch(ch) {
> + case '?':
what about "-h" (and ideally "--help" too) ?
> + default:
> + free(p);
> + usage();
> + }
> +
> + argc -= optind;
> + argv += optind;
> +
> + fp = stdin;
> + filename = "stdin";
> + rval = 0;
rval = EXIT_SUCCESS;
> + do {
> + if (*argv) {
> + if ((fp = fopen(*argv, "r")) == NULL) {
> + warn("cannot open %s", *argv );
> + rval = 1;
> + ++argv;
> + continue;
> + }
> + filename = *argv++;
> + }
> +
> + while (fgetws(p, buflen, fp)) {
> +
> + len = wcslen(p);
> +
> + /* This is my hack from setpwnam.c -janl */
> + while (p[len-1] != '\n' && !feof(fp)) {
> + /* Extend input buffer if it failed getting the whole line */
> +
> + /* So now we double the buffer size */
> + buflen *= 2;
> +
> + p = realloc(p, buflen*sizeof(wchar_t));
> + if (p == NULL)
> + err(1, _("unable to allocate bufferspace"));
> +
> + /* And fill the rest of the buffer */
> + if (fgetws(&p[len], buflen/2, fp) == NULL) break;
> +
> + len = wcslen(p);
> +
> + /* That was a lot of work for nothing. Gimme perl! */
> + }
> +
> + t = p + len - 1 - (*(p+len-1)=='\r' || *(p+len-1)=='\n');
> + for ( ; t >= p; --t)
> + if (*t != 0)
> + putwchar(*t);
> + putwchar('\n');
> + }
> + fflush(fp);
> + if (ferror(fp)) {
> + warn("%s", filename);
> + rval = 1;
rval = EXIT_FAILURE;
> + }
> + if (fclose(fp))
> + rval = 1;
> + } while(*argv);
> +
> + free(p);
> +
> + exit(rval);
return rval
> +}
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
