[patch] add function for system argument limit checking

Rafael EspĂ­ndola rafael.espindola at gmail.com
Tue Apr 9 17:51:22 PDT 2013


On 3 April 2013 16:59, Nathan Froyd <froydnj at mozilla.com> wrote:
> Hi!
>
> In:
>
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130311/075977.html
>
> I proposed a patch to fix PR 15171, a problem with invoking the linker with
> too many command-line arguments.  Rafael suggested that the right way to fix
> this sort of thing is to add bits in llvm/lib/Support/ for determining the
> maximum number of command-line arguments.
>
> The attached patch adds these bits.  On Unix, we consult sysconf(_SC_ARG_MAX)
> and tweak the numbers a bit to account for environment variables.  On
> Windows, we count up the size of the arguments, accounting for quoting,
> spaces between arguments, and the trailing NULL.


Thanks for the patch and so sorry for taking this long to review it.


+  bool ArgumentsFitWithinSystemLimits(const ArrayRef<const char*>& args);

We now start function names with a lowercase letter:
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

ArrayRef is semantically a const reference, so you can drop the
'const' and the '&'.

+  // Conservatively account for space required by environment variables.
+  ArgMax /= 2;
+
+  // +1 to account for the terminating NULL pointer.
+  return (args.size() + 1) < size_t(ArgMax);

No need to be precise after the /2 :-)

Don't you want to add the string length (strlen) of each arg?

> -Nathan

Cheers,
Rafael



More information about the llvm-commits mailing list