1.8.0: alloca not portable

[0-wiz-0]

We've updated libgit2 to 1.8.0 in pkgsrc. Now the build on Illumos fails with:

[ 29%] Building C object src/cli/CMakeFiles/git2_cli.dir/opt.c.o
/home/pbulk/build/devel/libgit2/work/libgit2-1.8.0/src/cli/opt.c: In function 'cli_opt_parse':
/home/pbulk/build/devel/libgit2/work/libgit2-1.8.0/src/cli/opt.c:560:23: warning: implicit declaration of function 'alloca'; did you mean 'valloc'? [-Wimplicit-function-declaration]
  560 |         given_specs = alloca(sizeof(const cli_opt_spec *) * (args_len + 1));
      |                       ^~~~~~
      |                       valloc
/home/pbulk/build/devel/libgit2/work/libgit2-1.8.0/src/cli/opt.c:560:21: warning: assignment to 'const cli_opt_spec **' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  560 |         given_specs = alloca(sizeof(const cli_opt_spec *) * (args_len + 1));
      |                     ^
At top level:
cc1: note: unrecognized command-line option '-Wno-documentation-deprecated-sync' may have been intended to silence earlier diagnostics
[ 29%] Building C object src/cli/CMakeFiles/git2_cli.dir/opt_usage.c.o
[ 29%] Building C object src/cli/CMakeFiles/git2_cli.dir/progress.c.o
[ 29%] Building C object src/cli/CMakeFiles/git2_cli.dir/unix/sighandler.c.o
[ 30%] Linking C executable ../../git2
Undefined			first referenced
 symbol  			    in file
alloca                              CMakeFiles/git2_cli.dir/opt.c.o
ld: fatal: symbol referencing errors. No output written to ../../git2
collect2: error: ld returned 1 exit status
*** [git2] Error code 1

The NetBSD man page lists a lot of limitations for alloca and recommend it not be used in portable code. Also, on NetBSD it's provided by stdlib.h, while on Illumos it's in alloca.h. The Illumos man page also strongly discourages its use.

[boretrk]

Yep, alloca is non-posix.
const cli_opt_spec *given_specs[args_len + 1]; would probably be a better option in this case.

Pretty much any argument that can be made against using VLA will apply to alloca, but VLA has the benefit of being part of C99.

[ethomson]

Do you know offhand whether MSVC supports VLA and if so, when that support started? alloca sort of sucks, but it's mostly portable. (Having to include alloc.h is hardly a deal-breaker. Once the AmigaOS folks start trying to use this, though, that's when we'll need to deal with this.)

Anyway, v1.8.1 should properly support Illumos, Solaris, SunOS and include alloc.h (@0-wiz-0 please let me know if it does not).

I'll leave this open since no, it's not really that portable, as the issue title describes. It's something to chew on but it's pretty low-priority.

[boretrk]

Oh, my bad, I honestly thought MSVC had full C99 support but it seems like they never made any such claim.
It does however seem like they have deprecated alloca but as you say it still compiles.

[ethomson]

They've been slowly chipping away at improving their C compiler. I'll take a look and see where VLA support starts and we can decide how we should attack this. (Could be ifdefs, even.)

Could also just do a malloc. Might be the simplest path forward.