On 03/13/16 13:31 , Sami Kerola wrote:
> The getopt(1) is short living command, and one could argue ensuring all
> allocations are freed at end of execution is waste of time. There is a
> point in that, but making test-suite runs to be less noisy with ASAN is also
> nice as it encourages reading the errors when/if they happen.
>
> Signed-off-by: Sami Kerola <kerolasa@xxxxxx>
> ---
> misc-utils/getopt.c | 36 ++++++++++++++++++++++++++++--------
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/misc-utils/getopt.c b/misc-utils/getopt.c
> index c4144f6..be2ed38 100644
> --- a/misc-utils/getopt.c
> +++ b/misc-utils/getopt.c
> @@ -79,13 +79,23 @@
> /* The shells recognized. */
> typedef enum { BASH, TCSH } shell_t;
>
> +/* This is a copy of getopt_long(3) structure, in which *name does not have
> + * const, so that is can be free'd at end of execution. */
> +struct getoption {
> + char *name;
> + int has_arg;
> + int *flag;
> + int val;
> +};
What will happen if some implementation will add new fields in `struct
option`? (Sure, unlikely, but).
IMO, `free((char *)option->name)` is *much* safer.
> struct getopt_control {
> shell_t shell; /* the shell we generate output for */
> char *optstr; /* getopt(3) optstring */
> - struct option *long_options; /* long options */
> + struct getoption *long_options; /* long options */
> int long_options_length; /* length of options array */
> int long_options_nr; /* number of used elements in array */
> unsigned int
> + free_name:1, /* free up argv[0] after printout */
> compatible:1, /* compatibility mode for 'difficult' programs */
> quiet_errors:1, /* print errors */
> quiet_output:1, /* print output */
> @@ -181,7 +191,7 @@ static void print_normalized(const struct getopt_control *ctl, const char *arg)
> * optstr must contain the short options, and longopts the long options.
> * Other settings are found in global variables.
> */
> -static int generate_output(const struct getopt_control *ctl, char *argv[], int argc)
> +static int generate_output(struct getopt_control *ctl, char *argv[], int argc)
> {
> int exit_code = EXIT_SUCCESS; /* Assume everything will be OK */
> int opt;
> @@ -195,8 +205,10 @@ static int generate_output(const struct getopt_control *ctl, char *argv[], int a
> optind = 0;
>
> while ((opt =
> - (getopt_long_fp(argc, argv, ctl->optstr, ctl->long_options, &longindex)))
> - != EOF)
> + (getopt_long_fp
> + (argc, argv, ctl->optstr,
> + (const struct option *)ctl->long_options, &longindex)))
> + != EOF) {
> if (opt == '?' || opt == ':')
> exit_code = GETOPT_EXIT_CODE;
> else if (!ctl->quiet_output) {
> @@ -216,13 +228,19 @@ static int generate_output(const struct getopt_control *ctl, char *argv[], int a
> print_normalized(ctl, optarg ? optarg : "");
> }
> }
> -
> + }
> if (!ctl->quiet_output) {
> printf(" --");
> while (optind < argc)
> print_normalized(ctl, argv[optind++]);
> printf("\n");
> }
> + for (longindex = 0; longindex < ctl->long_options_nr; longindex++)
> + free(ctl->long_options[longindex].name);
> + free(ctl->long_options);
> + free(ctl->optstr);
> + if (ctl->free_name)
> + free(argv[0]);
> return exit_code;
> }
>
> @@ -373,9 +391,6 @@ int main(int argc, char *argv[])
> textdomain(PACKAGE);
> atexit(close_stdout);
>
> - add_longopt(&ctl, NULL, 0); /* init */
> - getopt_long_fp = getopt_long;
> -
> if (getenv("GETOPT_COMPATIBLE"))
> ctl.compatible = 1;
>
> @@ -391,6 +406,9 @@ int main(int argc, char *argv[])
> parse_error(_("missing optstring argument"));
> }
>
> + add_longopt(&ctl, NULL, 0); /* init */
> + getopt_long_fp = getopt_long;
> +
> if (argv[1][0] != '-' || ctl.compatible) {
> ctl.quote = 0;
> ctl.optstr = xmalloc(strlen(argv[1]) + 1);
> @@ -417,6 +435,7 @@ int main(int argc, char *argv[])
> case 'n':
> free(name);
> name = xstrdup(optarg);
> + ctl.free_name = 1;
> break;
> case 'q':
> ctl.quiet_errors = 1;
> @@ -428,6 +447,7 @@ int main(int argc, char *argv[])
> ctl.shell = shell_type(optarg);
> break;
> case 'T':
> + free(ctl.long_options);
> return TEST_EXIT_CODE;
> case 'u':
> ctl.quote = 0;
> --
> 2.7.2
--
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
