On Friday 19 June 2009 01:07:04 Colin Watson wrote:
> I guess I'm a glutton for punishment today since y'all seem to have
> completely different approaches to things than I do based on comments on
> my patches, but hey, this one causes zero size change according to
> bloatcheck so maybe we can agree on this! ;-)
> 
> The Kickstart feature in the Debian/Ubuntu installer needs 'getopt -l'
> in order to work properly (or, at least, I'd have to write it in C
> otherwise, and I'd much prefer to write it in shell for a variety of
> good reasons that I don't need to go into here), but the only way to
> enable that is to enable long options for all of busybox. That's much
> bloatier than just turning it on for getopt, so I'd rather do that.
> 
> This adds a separate CONFIG_FEATURE_GETOPT_LONG knob that can be
> adjusted independently.
> 
> Signed-off-by: Colin Watson <[email protected]>
> ---
>  include/libbb.h      |    2 +-
>  libbb/getopt32.c     |   10 +++++-----
>  scripts/defconfig    |    1 +
>  util-linux/Config.in |    7 +++++++
>  util-linux/getopt.c  |   20 ++++++++++----------
>  5 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/include/libbb.h b/include/libbb.h
> index 62a60f9..fabfa4a 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -835,7 +835,7 @@ int sanitize_env_if_suid(void) FAST_FUNC;
>  
>  extern const char *const bb_argv_dash[]; /* "-", NULL */
>  extern const char *opt_complementary;
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_GETOPT_LONG || ENABLE_FEATURE_GETOPT_LONG
>  #define No_argument "\0"
>  #define Required_argument "\001"
>  #define Optional_argument "\002"
> diff --git a/libbb/getopt32.c b/libbb/getopt32.c
> index 1eb868c..2bb8b0a 100644
> --- a/libbb/getopt32.c
> +++ b/libbb/getopt32.c
> @@ -314,7 +314,7 @@ typedef struct {
>  } t_complementary;
>  
>  /* You can set applet_long_options for parse called long options */
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_GETOPT_LONG || ENABLE_FEATURE_GETOPT_LONG
>  static const struct option bb_null_long_options[1] = {
>       { 0, 0, 0, 0 }
>  };
> @@ -335,7 +335,7 @@ getopt32(char **argv, const char *applet_opts, ...)
>       const unsigned char *s;
>       t_complementary *on_off;
>       va_list p;
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_GETOPT_LONG || ENABLE_FEATURE_GETOPT_LONG
>       const struct option *l_o;
>       struct option *long_options = (struct option *) &bb_null_long_options;
>  #endif
> @@ -384,7 +384,7 @@ getopt32(char **argv, const char *applet_opts, ...)
>               c++;
>       }
>  
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_GETOPT_LONG || ENABLE_FEATURE_GETOPT_LONG
>       if (applet_long_options) {
>               const char *optstr;
>               unsigned i, count;
> @@ -424,7 +424,7 @@ getopt32(char **argv, const char *applet_opts, ...)
>   next_long: ;
>               }
>       }
> -#endif /* ENABLE_GETOPT_LONG */
> +#endif /* ENABLE_GETOPT_LONG || ENABLE_FEATURE_GETOPT_LONG */
>       for (s = (const unsigned char *)opt_complementary; s && *s; s++) {
>               t_complementary *pair;
>               unsigned *pair_switch;
> @@ -543,7 +543,7 @@ getopt32(char **argv, const char *applet_opts, ...)
>        * "fake" short options, like this one:
>        * wget $'-\203' "Test: test" http://kernel.org/
>        * (supposed to act as --header, but doesn't) */
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_GETOPT_LONG || ENABLE_FEATURE_GETOPT_LONG
>       while ((c = getopt_long(argc, argv, applet_opts,
>                       long_options, NULL)) != -1) {
>  #else
> diff --git a/scripts/defconfig b/scripts/defconfig
> index a863eca..ced02bc 100644
> --- a/scripts/defconfig
> +++ b/scripts/defconfig
> @@ -485,6 +485,7 @@ CONFIG_MKFS_MINIX=y
>  CONFIG_FEATURE_MINIX2=y
>  CONFIG_MKFS_VFAT=y
>  CONFIG_GETOPT=y
> +CONFIG_FEATURE_GETOPT_LONG=y
>  CONFIG_HEXDUMP=y
>  CONFIG_FEATURE_HEXDUMP_REVERSE=y
>  CONFIG_HD=y
> diff --git a/util-linux/Config.in b/util-linux/Config.in
> index 0245501..ba9ec06 100644
> --- a/util-linux/Config.in
> +++ b/util-linux/Config.in
> @@ -250,6 +250,13 @@ config GETOPT
>         written by others, this utility may be for you. Most people will
>         wisely leave this disabled.
>  
> +config FEATURE_GETOPT_LONG
> +     bool "Support option -l"
> +     default y if GETOPT_LONG
> +     help
> +       Enable support for recognising long options using the -l option to
> +       getopt.
> +
>  config HEXDUMP
>       bool "hexdump"
>       default n
> diff --git a/util-linux/getopt.c b/util-linux/getopt.c
> index fd67287..adb9675 100644
> --- a/util-linux/getopt.c
> +++ b/util-linux/getopt.c
> @@ -38,7 +38,7 @@
>     mode */
>  enum {
>       NON_OPT = 1,
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>  /* LONG_OPT is the code that is returned when a long option is found. */
>       LONG_OPT = 2
>  #endif
> @@ -53,7 +53,7 @@ enum {
>       OPT_s   = 0x10, // -s
>       OPT_T   = 0x20, // -T
>       OPT_u   = 0x40, // -u
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>       OPT_a   = 0x80, // -a
>       OPT_l   = 0x100, // -l
>  #endif
> @@ -141,7 +141,7 @@ static const char *normalize(const char *arg)
>   * optstr must contain the short options, and longopts the long options.
>   * Other settings are found in global variables.
>   */
> -#if !ENABLE_GETOPT_LONG
> +#if !ENABLE_FEATURE_GETOPT_LONG
>  #define generate_output(argv,argc,optstr,longopts) \
>       generate_output(argv,argc,optstr)
>  #endif
> @@ -149,7 +149,7 @@ static int generate_output(char **argv, int argc, const 
> char *optstr, const stru
>  {
>       int exit_code = 0; /* We assume everything will be OK */
>       int opt;
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>       int longindex;
>  #endif
>       const char *charptr;
> @@ -168,7 +168,7 @@ static int generate_output(char **argv, int argc, const 
> char *optstr, const stru
>  
>       while (1) {
>               opt =
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>                       alternative ?
>                       getopt_long_only(argc, argv, optstr, longopts, 
> &longindex) :
>                       getopt_long(argc, argv, optstr, longopts, &longindex);
> @@ -180,7 +180,7 @@ static int generate_output(char **argv, int argc, const 
> char *optstr, const stru
>               if (opt == '?' || opt == ':' )
>                       exit_code = 1;
>               else if (!quiet_output) {
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>                       if (opt == LONG_OPT) {
>                               printf(" --%s", longopts[longindex].name);
>                               if (longopts[longindex].has_arg)
> @@ -209,7 +209,7 @@ static int generate_output(char **argv, int argc, const 
> char *optstr, const stru
>       return exit_code;
>  }
>  
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>  /*
>   * Register several long options. options is a string of long options,
>   * separated by commas or whitespace.
> @@ -273,7 +273,7 @@ static void set_shell(const char *new_shell)
>   *   4) Returned for -T
>   */
>  
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>  static const char getopt_longopts[] ALIGN1 =
>       "options\0"      Required_argument "o"
>       "longoptions\0"  Required_argument "l"
> @@ -295,7 +295,7 @@ int getopt_main(int argc, char **argv)
>       unsigned opt;
>       const char *compatible;
>       char *s_arg;
> -#if ENABLE_GETOPT_LONG
> +#if ENABLE_FEATURE_GETOPT_LONG
>       struct option *long_options = NULL;
>       llist_t *l_arg = NULL;
>  #endif
> @@ -321,7 +321,7 @@ int getopt_main(int argc, char **argv)
>               return generate_output(argv+1, argc-1, s, long_options);
>       }
>  
> -#if !ENABLE_GETOPT_LONG
> +#if !ENABLE_FEATURE_GETOPT_LONG
>       opt = getopt32(argv, "+o:n:qQs:Tu", &optstr, &name, &s_arg);
>  #else
>       applet_long_options = getopt_longopts;

Hi,
maybe a more descriptive ENABLE_XX flag would be better so to avoid 
confusion about what is what. Maybe ENABLE_FEATURE_GETOPT_APP_LONGOPTS.

Just my 0,2 € cents.

Ciao,
Tito
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to