[patch] add function for system argument limit checking

Nathan Froyd froydnj at mozilla.com
Wed Apr 10 08:44:34 PDT 2013


On Tue, Apr 09, 2013 at 08:51:22PM -0400, Rafael EspĂ­ndola wrote:
> On 3 April 2013 16:59, Nathan Froyd <froydnj at mozilla.com> wrote:
> Thanks for the patch and so sorry for taking this long to review it.

Thanks for the review.

> +  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

Ah, the perils of following surrounding code conventions.

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

Done.

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

I was a bit confused by all the documentation I could find on ARG_MAX;
everything talked about the "total length" of argv and it wasn't clear
that meant adding up all the string lengths.  But wading through the
Linux kernel source code and some of the headers confirms that summing
up the lengths of all the arguments is really what matters (and makes
more sense).  So this revised patch does that, still being precise. ;)

-Nathan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arguments-system-limit.patch
Type: text/x-diff
Size: 2497 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130410/a3169deb/attachment.patch>


More information about the llvm-commits mailing list