Re: [PATCH 3/3] ash: exec: -a option for setting zeroth arg

2017-04-12 Thread Kaarle Ritvanen 
On Wed, 12 Apr 2017, Denys Vlasenko wrote:

> I committed a change which implements "exec -a"
> in a slightly different way. Please try current git.

Works for me. Thanks a lot!

BR,
Kaarle
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 3/3] ash: exec: -a option for setting zeroth arg

2017-04-12 Thread Denys Vlasenko 
On Tue, Apr 11, 2017 at 11:58 PM, Kaarle Ritvanen
 wrote:
> -execcmd(int argc UNUSED_PARAM, char **argv)
> +execcmd(int argc, char **argv)
>  {
> -   if (argv[1]) {
> +   int opt;
> +   char *argv0 = NULL;
> +   char *cmdname = NULL;
> +
> +   GETOPT_RESET
> +   while ((opt = getopt(argc, argv, "a:")) != -1)

(1) The rest of ash does not use getopt().

(2) Did you test this with, say, "exec stty -a" command?

I committed a change which implements "exec -a"
in a slightly different way. Please try current git.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 2/3] ash: shellexec: rename variable

2017-04-12 Thread Denys Vlasenko 
On Tue, Apr 11, 2017 at 11:58 PM, Kaarle Ritvanen
 wrote:
> Signed-off-by: Kaarle Ritvanen 
> ---
>  shell/ash.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index 983f7b1..58ae950 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -7748,7 +7748,7 @@ static void shellexec(char **, const char *, int) 
> NORETURN;
>  static void
>  shellexec(char **argv, const char *path, int idx)
>  {
> -   char *cmdname;
> +   char *cmdpath;

This would introduce gratuitous difference from dash source.
Let's not do that. dash often have patches worth backporting,
and every difference makes backporting harder.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/3] libbb: GETOPT_RESET macro

2017-04-12 Thread Denys Vlasenko 
On Tue, Apr 11, 2017 at 11:58 PM, Kaarle Ritvanen
 wrote:
> Signed-off-by: Kaarle Ritvanen 
> ---
>  include/libbb.h| 19 +++
>  libbb/getopt32.c   |  8 +---
>  libbb/vfork_daemon_rexec.c | 28 ++--
>  runit/sv.c |  7 +--
>  shell/shell_common.c   |  8 +---
>  util-linux/getopt.c|  7 +--
>  6 files changed, 25 insertions(+), 52 deletions(-)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index 0407163..eb78619 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -1178,6 +1178,25 @@ extern uint32_t option_mask32;
>  extern uint32_t getopt32(char **argv, const char *applet_opts, ...) 
> FAST_FUNC;
>
>
> +/* BSD-derived getopt() functions require that optind be set to 1 in
> + * order to reset getopt() state.  This used to be generally accepted
> + * way of resetting getopt().  However, glibc's getopt()
> + * has additional getopt() state beyond optind, and requires that
> + * optind be set to zero to reset its state.  So the unfortunate state of
> + * affairs is that BSD-derived versions of getopt() misbehave if
> + * optind is set to 0 in order to reset getopt(), and glibc's getopt()
> + * will core dump if optind is set 1 in order to reset getopt().
> + *
> + * More modern versions of BSD require that optreset be set to 1 in
> + * order to reset getopt().  Sigh.  Standards, anyone?
> + */
> +#ifdef __GLIBC__
> +#define GETOPT_RESET optind = 0;
> +#else /* BSD style */
> +#define GETOPT_RESET optind = 1;
> +#endif

I think function-style macro is better:

+#ifdef __GLIBC__
+#define GETOPT_RESET() (optind = 0)
+#else /* BSD style */
+#define GETOPT_RESET() (optind = 1)
+#endif

Applied with this edit. Thanks.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 3/3] ash: exec: -a option for setting zeroth arg

2017-04-12 Thread Kaarle Ritvanen 
On Wed, 12 Apr 2017, Kang-Che Sung wrote:

> Back in January 2017, a person named Patrick Pief has suggested a similar 
> patch
> to add "exec -a" support:
> 
> http://lists.busybox.net/pipermail/busybox/2017-January/085146.html
> 
> Is your patch same as his or is there any difference?

My patch parses the '-a' option in execcmd, thus not affecting the 
behavior of shellexec when called from evalcommand. In addition, my patch 
uses getopt, making it a bit easier to add more options later.

Is there any reason not to merge my or Patrick's patch? If you prefer my 
patch, I can make the changes requested by Bartosz.

BR,
Kaarle
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 3/3] ash: exec: -a option for setting zeroth arg

2017-04-12 Thread Kang-Che Sung 
Back in January 2017, a person named Patrick Pief has suggested a similar patch
to add "exec -a" support:

http://lists.busybox.net/pipermail/busybox/2017-January/085146.html

Is your patch same as his or is there any difference?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/3] libbb: GETOPT_RESET macro

2017-04-12 Thread Bartosz Gołaszewski 
2017-04-11 23:58 GMT+02:00 Kaarle Ritvanen :
> Signed-off-by: Kaarle Ritvanen 
> ---
>  include/libbb.h| 19 +++
>  libbb/getopt32.c   |  8 +---
>  libbb/vfork_daemon_rexec.c | 28 ++--
>  runit/sv.c |  7 +--
>  shell/shell_common.c   |  8 +---
>  util-linux/getopt.c|  7 +--
>  6 files changed, 25 insertions(+), 52 deletions(-)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index 0407163..eb78619 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -1178,6 +1178,25 @@ extern uint32_t option_mask32;
>  extern uint32_t getopt32(char **argv, const char *applet_opts, ...) 
> FAST_FUNC;
>
>
> +/* BSD-derived getopt() functions require that optind be set to 1 in
> + * order to reset getopt() state.  This used to be generally accepted
> + * way of resetting getopt().  However, glibc's getopt()
> + * has additional getopt() state beyond optind, and requires that
> + * optind be set to zero to reset its state.  So the unfortunate state of
> + * affairs is that BSD-derived versions of getopt() misbehave if
> + * optind is set to 0 in order to reset getopt(), and glibc's getopt()
> + * will core dump if optind is set 1 in order to reset getopt().
> + *
> + * More modern versions of BSD require that optreset be set to 1 in

s/optreset/optind

> + * order to reset getopt().  Sigh.  Standards, anyone?
> + */
> +#ifdef __GLIBC__
> +#define GETOPT_RESET optind = 0;
> +#else /* BSD style */
> +#define GETOPT_RESET optind = 1;
> +#endif
> +

This should be

#define GETOPT_RESET() do { \
optind = 0; \
} while (0)

> +
>  /* Having next pointer as a first member allows easy creation
>   * of "llist-compatible" structs, and using llist_FOO functions
>   * on them.
> diff --git a/libbb/getopt32.c b/libbb/getopt32.c
> index 497fc01..38fac14 100644
> --- a/libbb/getopt32.c
> +++ b/libbb/getopt32.c
> @@ -576,13 +576,7 @@ getopt32(char **argv, const char *applet_opts, ...)
>  * run_nofork_applet() does this, but we might end up here
>  * also via gunzip_main() -> gzip_main(). Play safe.
>  */
> -#ifdef __GLIBC__
> -   optind = 0;
> -#else /* BSD style */
> -   optind = 1;
> -   /* optreset = 1; */
> -#endif
> -   /* optarg = NULL; opterr = 0; optopt = 0; - do we need this?? */
> +   GETOPT_RESET
>
> /* Note: just "getopt() <= 0" will not work well for
>  * "fake" short options, like this one:
> diff --git a/libbb/vfork_daemon_rexec.c b/libbb/vfork_daemon_rexec.c
> index 2e7dc2d..e2a6b39 100644
> --- a/libbb/vfork_daemon_rexec.c
> +++ b/libbb/vfork_daemon_rexec.c
> @@ -121,28 +121,8 @@ int FAST_FUNC run_nofork_applet(int applet_no, char 
> **argv)
>
> /* In case getopt() or getopt32() was already called:
>  * reset the libc getopt() function, which keeps internal state.
> -*
> -* BSD-derived getopt() functions require that optind be set to 1 in
> -* order to reset getopt() state.  This used to be generally accepted
> -* way of resetting getopt().  However, glibc's getopt()
> -* has additional getopt() state beyond optind, and requires that
> -* optind be set to zero to reset its state.  So the unfortunate 
> state of
> -* affairs is that BSD-derived versions of getopt() misbehave if
> -* optind is set to 0 in order to reset getopt(), and glibc's getopt()
> -* will core dump if optind is set 1 in order to reset getopt().
> -*
> -* More modern versions of BSD require that optreset be set to 1 in
> -* order to reset getopt().  Sigh.  Standards, anyone?
>  */
> -#ifdef __GLIBC__
> -   optind = 0;
> -#else /* BSD style */
> -   optind = 1;
> -   /* optreset = 1; */
> -#endif
> -   /* optarg = NULL; opterr = 1; optopt = 63; - do we need this too? */
> -   /* (values above are what they initialized to in glibc and uclibc) */
> -   /* option_mask32 = 0; - not needed, no applet depends on it being 0 */
> +   GETOPT_RESET
>
> argc = 1;
> while (argv[argc])
> @@ -167,11 +147,7 @@ int FAST_FUNC run_nofork_applet(int applet_no, char 
> **argv)
> restore_nofork_data();
>
> /* Other globals can be simply reset to defaults */
> -#ifdef __GLIBC__
> -   optind = 0;
> -#else /* BSD style */
> -   optind = 1;
> -#endif
> +   GETOPT_RESET
>
> return rc & 0xff; /* don't confuse people with "exitcodes" >255 */
>  }
> diff --git a/runit/sv.c b/runit/sv.c
> index 9e21322..4ba5df7 100644
> --- a/runit/sv.c
> +++ b/runit/sv.c
> @@ -688,12 +688,7 @@ int svc_main(int argc UNUSED_PARAM, char **argv)
> /* getopt32() was already called:
>  * reset the libc getopt() function, which keeps internal state.
>  */
> -#ifdef __GLIBC__
> -   optind = 0;
> -#else /* BSD style */
> -   optind = 1;