When an option that requires an argument is the last character of argv[argc-1], getopt computes argv[argc] + optpos. While optpos is always zero in this case, adding it to null pointer is still undefined. --- src/misc/getopt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/misc/getopt.c b/src/misc/getopt.c index c3f66995..af12973a 100644 --- a/src/misc/getopt.c +++ b/src/misc/getopt.c @@ -87,7 +87,8 @@ int getopt(int argc, char * const argv[], const char *optstring) if (optstring[i] == ':') { optarg = 0; if (optstring[i+1] != ':' || optpos) { - optarg = argv[optind++] + optpos; + optarg = argv[optind++]; + if (optarg) optarg += optpos; optpos = 0; } if (optind > argc) { -- 2.39.1
Hi,
On Fri, 10 Mar 2023, Alexey Izbyshev wrote:
> When an option that requires an argument is the last character of
> argv[argc-1], getopt computes argv[argc] + optpos. While optpos
> is always zero in this case, adding it to null pointer is still
> undefined.
> ---
> src/misc/getopt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/misc/getopt.c b/src/misc/getopt.c
> index c3f66995..af12973a 100644
> --- a/src/misc/getopt.c
> +++ b/src/misc/getopt.c
> @@ -87,7 +87,8 @@ int getopt(int argc, char * const argv[], const char *optstring)
> if (optstring[i] == ':') {
> optarg = 0;
> if (optstring[i+1] != ':' || optpos) {
> - optarg = argv[optind++] + optpos;
> + optarg = argv[optind++];
> + if (optarg) optarg += optpos;
Can this be written as 'if (optpos) optarg += optpos;' instead? That will be
folded back into plain addition by the compiler.
(also (unlike the quoted variant) would allow undefined behavior
instrumentation to catch attempted NULL pointer arithmetic)
Alexander
On 2023-03-10 19:28, Alexander Monakov wrote: > Hi, > > On Fri, 10 Mar 2023, Alexey Izbyshev wrote: > >> When an option that requires an argument is the last character of >> argv[argc-1], getopt computes argv[argc] + optpos. While optpos >> is always zero in this case, adding it to null pointer is still >> undefined. >> --- >> src/misc/getopt.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/src/misc/getopt.c b/src/misc/getopt.c >> index c3f66995..af12973a 100644 >> --- a/src/misc/getopt.c >> +++ b/src/misc/getopt.c >> @@ -87,7 +87,8 @@ int getopt(int argc, char * const argv[], const char >> *optstring) >> if (optstring[i] == ':') { >> optarg = 0; >> if (optstring[i+1] != ':' || optpos) { >> - optarg = argv[optind++] + optpos; >> + optarg = argv[optind++]; >> + if (optarg) optarg += optpos; > > Can this be written as 'if (optpos) optarg += optpos;' instead? That > will be > folded back into plain addition by the compiler. > Yes, "if (optpos) ..." is actually what I initially wrote before changing it to the submitted variant. I'm fine with changing it back; thanks for the codegen check. > (also (unlike the quoted variant) would allow undefined behavior > instrumentation to catch attempted NULL pointer arithmetic) > Yes, a good point too. Alexey