[LLVMdev] Some fixes for the GetMainExecutable function

[Kees van Reeuwijk]

On 09/02/2010 07:51 PM, Dan Gohman wrote:
>
> On Sep 2, 2010, at 10:03 AM, Kees van Reeuwijk wrote:
>
>> -  snprintf(buf, PATH_MAX, "%s//%s", dir, bin);
>
> There's no comment, and no mention in the commit message.
> It seems pretty safe to fix.
>
>> Also, the if-all-else-fails return value of GetMainExecutable is an empty path, but since the users of this function don't test for that, this choice leads to rather obscure errors. Wouldn't it be better to give an explicit error in such cases,
>
> Issuing an error is hard to do right here, since it's in such a low-level
> library. However, it doesn't have very many users, and at least some of
> them already do handle this error gracefully, so can any users which
> don't be fixed?


It depends a bit on the nature of the error state. If it were my own program, and if this is never supposed to happen (which seems 
to be the case here) I would print an error message starting with "Internal error:", and probably do an exit(1).

>> or at the very least to return a path just containing Arg0 rather than the empty string?
>
> If GetMainExecutable ever were to actually fail, it would mean that
> something is seriously wrong with the system, so it's quite acceptable
> to just fail, rather than cast about.

Ok, but then it would be entirely reasonable (I would even argue mandatory) to emit an error message in GetMainExecutable, and 
probably make the error fatal.

And to take it one step further: the current code silently just returns an empty path on platforms that do not match any of the 
#ifdefs in this function. I think people porting llvm to a new platform would very much appreciate a

#else
#error ...

so that they don't get bitten by this.

But perhaps I'm misunderstanding the 'contract' of this function, and is it allowed to fail after all. (In which case the clang 
driver should test the result of this function.)

-- 
Dr. Ir. Kees van Reeuwijk, Vrije Universiteit Amsterdam