On Fri, May 29, 2020 at 12:45:12PM +0200, Otto Moerbeek wrote:
[...]
> This fails on arm64:
>
> /usr/src/usr.bin/ssh/ssh/../misc.c:1181:51: error: passing 'void *' to
> parameter of incompatible type
> 'va_list' (aka '__builtin_va_list')
> ret = vdollar_percent_expand(&err, 1, 0, string, NULL);
> ^~~~
> /usr/include/sys/_null.h:10:14: note: expanded from macro 'NULL'
> #define NULL ((void *)0)
> ^~~~~~~~~~~
> /usr/src/usr.bin/ssh/ssh/../misc.c:1060:33: note: passing argument to
> parameter 'ap' here
> const char *string, va_list ap)
> ^
> 1 error generated.
Sigh, I just found that out in portable too. Here's the message I just
sent to the other openssh developers. If you have any better
suggestions or an ok I'd be happy to hear it.
---------------------------------------------------------------------
A brief recap:
- percent_expand originally was variadic, which made sense since it
could take a variable number of %-tokens.
- When we added environment variables, we put it in the same function as
percent_expand instead of a separate one to prevent unanticipated
interactions between the two. this became vdollar_percent_expand.
- There's wrapper functions around that to do percent, dollar or both
expansions.
- dollar_expand is not variadic because the variadic args are for
percent expansion which it doesn't do.
- originally I passed a NULL as the va_list to vdollar_percent_expand.
Damien pointed out that there's no guarantee va_list is a pointer
equivalent, so it was changed to a real va_list which was memset-0ed.
After commiting, I found out that this memset blew up on i386:
misc.c:1182:23: error: ´memset´ call operates on objects of type
´char´ while the size is based on a different type ´va_list´ (´char *´)
[-Werror,-Wsizeof-pointer-memaccess]
memset(ap, 0, sizeof(ap)); /* unused */
To unbreak the build, I switched it back to passing NULL, which works on at
least i386 and amd64. Now I'm trying to fix the mess properly.
As far as I can tell, there's no standards-compliant way to either pass
an unused va_list or initialize an unused real va_list. Googling
indicates that calling va_start from a non-variadic function will
generate a compiler warning, although our clang seems OK with it.
This diff below makes dollar_expand variadic, with any such arguments
ignored in the workhorse function.
Does anyone know a better way to fix this? Passing an uninitialized
va_list seems wrong even if the called function doesn't use it.
Index: misc.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.c,v
retrieving revision 1.151
diff -u -p -r1.151 misc.c
--- misc.c 29 May 2020 09:02:44 -0000 1.151
+++ misc.c 29 May 2020 09:41:47 -0000
@@ -1172,13 +1172,22 @@ vdollar_percent_expand(int *parseerror,
#undef EXPAND_MAX_KEYS
}
+/*
+ * Expand only environment variables.
+ * Note that although this function is variadic like the other similar
+ * functions, any such arguments will be unused.
+ */
+
char *
-dollar_expand(int *parseerr, const char *string)
+dollar_expand(int *parseerr, const char *string, ...)
{
char *ret;
int err;
+ va_list ap;
- ret = vdollar_percent_expand(&err, 1, 0, string, NULL);
+ va_start(ap, string);
+ ret = vdollar_percent_expand(&err, 1, 0, string, ap);
+ va_end(ap);
if (parseerr != NULL)
*parseerr = err;
return ret;
Index: misc.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.h,v
retrieving revision 1.86
diff -u -p -r1.86 misc.h
--- misc.h 29 May 2020 04:25:40 -0000 1.86
+++ misc.h 29 May 2020 09:41:48 -0000
@@ -69,7 +69,7 @@ long convtime(const char *);
const char *fmt_timeframe(time_t t);
char *tilde_expand_filename(const char *, uid_t);
-char *dollar_expand(int *, const char *string);
+char *dollar_expand(int *, const char *string, ...);
char *percent_expand(const char *, ...) __attribute__((__sentinel__));
char *percent_dollar_expand(const char *, ...) __attribute__((__sentinel__));
char *tohex(const void *, size_t);
--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
