On Mon, Jan 03, 2011 at 09:42:04PM +0100, Sami Kerola wrote:
> @@ -83,8 +97,8 @@ void *our_malloc(size_t size)
> {
> void *ret=malloc(size);
> if (! ret) {
> - fprintf(stderr,_("%s: Out of memory!\n"),"getopt");
> - exit(3);
> + fprintf(stderr,_("%s: Out of memory!\n"),progname);
> + exit(EXIT_INTERNAL_ERROR);
> }
> return(ret);
> }
> @@ -93,22 +107,20 @@ void *our_realloc(void *ptr, size_t size)
> {
> void *ret=realloc(ptr,size);
> if (! ret && size) {
> - fprintf(stderr,_("%s: Out of memory!\n"),"getopt");
> - exit(3);
> + fprintf(stderr,_("%s: Out of memory!\n"),progname);
> + exit(EXIT_INTERNAL_ERROR);
> }
> return(ret);
> }
Please, see include/xalloc.h. It'd be better to replace our_realloc()
and our_malloc() by xalloc and xrealloc.
> @@ -225,9 +233,9 @@ int generate_output(char * argv[],int argc,const char *optstr,
> void parse_error(const char *message)
> {
> if (message)
> - fprintf(stderr,"getopt: %s\n",message);
> - fputs(_("Try `getopt --help' for more information.\n"),stderr);
> - exit(2);
> + fprintf(stderr,"%s: %s\n",progname,message);
> + fprintf(stderr,_("Try `%s --help' for more information.\n"),progname);
> + exit(EXIT_PARSING);
> }
Please, "man errx" -- it's better to use functions from err.h.
> static struct option *long_options=NULL;
> @@ -317,9 +325,9 @@ void set_shell(const char *new_shell)
>
> void print_help(void)
> {
> - fputs(_("Usage: getopt optstring parameters\n"),stderr);
> - fputs(_(" getopt [options] [--] optstring parameters\n"),stderr);
> - fputs(_(" getopt [options] -o|--options optstring [options] [--]\n"),stderr);
> + fprintf(stderr,_("Usage: %s optstring parameters\n"),progname);
> + fprintf(stderr,_(" %s [options] [--] optstring parameters\n"),progname);
> + fprintf(stderr,_(" %s [options] -o|--options optstring [options] [--]\n"),progname);
Please, see usage() from some others utils (e.g. misc-utils/lsblk.c),
%1$s (e.g. sys-utils/flock.c) and program_invocation_short_name.
> case 'V':
> - printf(_("getopt (enhanced) 1.1.4\n"));
> - exit(0);
> + printf(_("getopt (%s)\n"),progname,PACKAGE_STRING);
Please, use:
printf(_("%s from %s"), program_invocation_short_name, PACKAGE_STRING);
> + exit_code = generate_output(argv+optind-1,argc-optind+1,optstr,long_options);
> + if (optstr)
> + free(optstr);
Please, don't use if-before-free in your code. It's unnecessary.
> + if (name)
> + free(name);
> + if (0 < long_options_nr) {
> + for (int i = long_options_nr; 0 <= i; i--)
> + free((void *) long_options[i].name);
> + free(long_options);
> + }
> + exit(exit_code);
> }
This all is unnecessary, use
return generate_output(argv+optind-1,argc-optind+1,optstr,long_options);
kernel deallocates all the things.
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
