Hi! On Wed, Jan 04, 2023 at 10:12:36PM +0800, Herbert Xu wrote: > On Wed, Jan 04, 2023 at 01:54:21PM +0100, наб wrote: > > AFAICT, after trawling through the Issue 8 Draft 2.1, > > there are no special provisions for unset/OPTIND, > Well it says this regarding OPTIND: > > : If the application sets OPTIND to the value 1, a new set of parameters > : can be used: either the current positional parameters or new arg > : values. Any other attempt to invoke getopts multiple times in a single > : shell execution environment with parameters (positional parameters > : or arg operands) that are not the same in all invocations, or with an > : OPTIND value modified to be a value other than 1, produces unspecified > : results. > > Unsetting OPTIND ought to do the same thing as setting it to "", On the basis of? Those are fundamentally unrelated operations, and when they are to do the same thing, this is explicitly noted. Additionally, this is a description of /getopts/, and therefore only applies if you do "OPTIND=dupa; getopts ..." ‒ the getopts call is unspecified. /Any/ operation on OPTIND is legal, and the current behaviour is wrong (but it seems that no-one has actually cared) ‒ POSIX defines this in terms of getopts inspecting OPTIND when it's run. "OPTIND=whatever" is legal and must result in OPTIND becoming whatever. bash, ksh, and zsh all reduce it to OPTIND=0, so it appears that no-one actually cares about this so erroring out is fine, I guess. (mksh does the same but folds to 1; posh refuses the assignment), "unset OPTIND" is legal, and subsequent getopts calls are well-defined (it's unclear to me what it oughta do /per se/, but it's not "been set", so it's unclear what it's well-defined /to/, but since "the index of the next argument to be processed in the shell variable OPTIND" and that value doesn't exist, erroring out is fine I guess), and all the above shells agree (as does mksh; posh ignores the unset). and this fell out of a debbug where someone used it (and additionally cites a few more shells), so people actually care about this behaviour. In testing the poster's shells, I found that yash behaves close to correct here: -- >8 -- nabijaczleweli@tarta dash 2$ OPTIND=dupa nabijaczleweli@tarta dash 2$ getopts a a a getopts: $OPTIND has an invalid value nabijaczleweli@tarta dash 1 2$ OPTIND=300 nabijaczleweli@tarta dash 2$ getopts a a a nabijaczleweli@tarta dash 1 2$ unset OPTIND nabijaczleweli@tarta dash 2$ set | grep -i optind nabijaczleweli@tarta dash 1 2$ getopts a a a getopts: $OPTIND has an invalid value -- >8 -- As does busybox ash: -- >8 -- ~/code/dash $ OPTIND=dupa ~/code/dash $ set | grep -i optind OPTIND='dupa' ~/code/dash $ getopts a a a ~/code/dash $ set | grep -i optind OPTIND='1' ~/code/dash $ unset OPTIND ~/code/dash $ set | grep -i optind _='OPTIND' ~/code/dash $ getopts a a a ~/code/dash $ set | grep -i optind OPTIND='1' -- >8 -- The minimal change required to achieve compatibility is my patch, or some equivalent variant of it, like passing NULL instead of flags. The minimal change required to achieve conformance would be to get parse OPTIND in getopts like yash. There's code in the wild that uses "unset OPTIND; getopts ..." instead of "OPTIND=1; getopts ...": https://codesearch.debian.net/search?q=unset+OPTIND&literal=1&perpkg=1 but those are all bash programs, so it's up to you whether they should receive explicit provisions for this. I'm including a PoC scissor-patch below; in particular the ternary should either be turned into an equivalent of ': "1"' if you want to support "unset OPTIND; getopts ..." or produce a better error than "./dash: 1: getopts: Illegal number: unset", but you get the idea (and shellparam should get fully boolified and dash.1 wants to have an explicit note that reset is OPTIND=1, but I can follow up with that if we're in agreement here). Best, -- >8 -- Subject: [PATCH] options: lazy-parse OPTIND in getopts, per POSIX This allows "unset OPTIND" and "OPTIND=dupa" to execute as-written (and as required by POSIX), and catches them as an error in getopts, which exits 2 ("An error occurred."). Fixes: https://bugs.debian.org/985478 --- src/options.c | 30 +++++++++++++++++------------- src/options.h | 5 ++++- src/var.h | 1 + 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/options.c b/src/options.c index a46c23b..578a13f 100644 --- a/src/options.c +++ b/src/options.c @@ -164,7 +164,7 @@ setarg0: shellparam.p = xargv; shellparam.optind = 1; shellparam.optoff = -1; - /* assert(shellparam.malloc == 0 && shellparam.nparam == 0); */ + /* assert(!shellparam.malloc && !shellparam.optreparse && shellparam.nparam == 0); */ while (*xargv) { shellparam.nparam++; xargv++; @@ -390,11 +390,9 @@ setcmd(int argc, char **argv) void -getoptsreset(value) - const char *value; +getoptsreset(const char *value) { - shellparam.optind = number(value) ?: 1; - shellparam.optoff = -1; + shellparam.optreparse = true; } /* @@ -408,22 +406,28 @@ int getoptscmd(int argc, char **argv) { char **optbase; + unsigned max; if (argc < 3) sh_error("Usage: getopts optstring var [arg]"); else if (argc == 3) { optbase = shellparam.p; - if ((unsigned)shellparam.optind > shellparam.nparam + 1) { - shellparam.optind = 1; - shellparam.optoff = -1; - } + max = shellparam.nparam + 1; } else { optbase = &argv[3]; - if ((unsigned)shellparam.optind > argc - 2) { - shellparam.optind = 1; - shellparam.optoff = -1; - } + max = argc - 2; + } + + if (shellparam.optreparse) { + shellparam.optind = number(optindset() ? optindval() : "unset") ?: 1; + shellparam.optoff = -1; + shellparam.optreparse = false; + } + + if ((unsigned)shellparam.optind > max) { + shellparam.optind = 1; + shellparam.optoff = -1; } return getopts(argv[1], argv[2], optbase); diff --git a/src/options.h b/src/options.h index 975fe33..8aebf9f 100644 --- a/src/options.h +++ b/src/options.h @@ -34,9 +34,12 @@ * @(#)options.h 8.2 (Berkeley) 5/4/95 */ +#include <stdbool.h> + struct shparam { int nparam; /* # of positional parameters (without $0) */ - unsigned char malloc; /* if parameter list dynamically allocated */ + bool malloc; /* if parameter list dynamically allocated */ + bool optreparse; /* re-parse optind/optoff from $OPTIND */ char **p; /* parameter list */ int optind; /* next parameter to be processed by getopts */ int optoff; /* used by getopts */ diff --git a/src/var.h b/src/var.h index aa7575a..ca90a68 100644 --- a/src/var.h +++ b/src/var.h @@ -133,6 +133,7 @@ extern char linenovar[]; #define attyset() ((vatty.flags & VUNSET) == 0) #endif #define mpathset() ((vmpath.flags & VUNSET) == 0) +#define optindset() ((voptind.flags & VUNSET) == 0) void initvar(void); struct var *setvar(const char *name, const char *val, int flags); -- 2.30.2
Attachment:
signature.asc
Description: PGP signature
