[PATCH][llvm-c] expose sys::getProcessTriple

Filip Pizlo fpizlo at apple.com
Mon Jul 1 12:44:06 PDT 2013


On Jul 1, 2013, at 10:39 AM, Jim Grosbach <grosbach at apple.com> wrote:

> Hi Filip,
> 
> This seems generally fine. Just a couple of nitpicks.
> 
> Let’s make it explicit that it’s the host process string being retrieved since this will be used mainly in the MCJIT context and that it’s a triple being retrieved. "LLVMGetProcessTriple()” seems right to me.

OK.

> 
> Less important, but I’d prefer having the size argument to the function say that’s what it is, so something like, “BufferSize,” perhaps?

Good idea.

> 
>> + * size_t targetSize = LLVMGetProcessTarget(0, 0);
> 
> 
> Use an explicit NULL here for the pointer value.

Will do.

> 
>> "If TargetBytes isn't enough then the Target string is filled only up to TargetBytes; that is this function will never overflow that bound.”
> 
> 
> Is the string null terminated when there’s not enough room for the whole string? IMO it should be.

Yes.  The string you get back is always null terminated.

> 
> -Jim
> 
> 
> 
> On Jun 28, 2013, at 2:15 PM, Filip Pizlo <fpizlo at apple.com> wrote:
> 
>> Ping!
>> 
>> -F
>> 
>> 
>> On Jun 27, 2013, at 8:05 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>> 
>>> The fact that this isn't already exposed makes using some other C APIs, like the disassembler API, really awkward in a JIT context - you don't want to have to describe to LLVM what the target name is for the process you're in, when LLVM already has this information.
>>> 
>>> The challenge here is that sys::getProcessTriple() returns a std::string that it allocates itself.  So we can't just have a "LLVMGetProcessTarget()" that returns a char*, unless we then require the user to free() that char*.  I considered that but I feared that it would have been confusing to users: for example LLVMGetTarget() returns a char* that doesn't require deallocation.  So, to make the ownership rules more explicit, I made the API take an already allocated char* and a size.
>>> 
>>> I wanted to allow for this idiom for people who don't want to guess the right string size:
>>> 
>>> size_t targetSize = LLVMGetProcessTarget(0, 0);
>>> char *target = malloc(targetSize);
>>> LLVMGetProcessTarget(target, targetSize);
>>> 
>>> This means that I couldn't just call snprintf().  I actually ended up getting the buffer allocation wrong on the first try, so I include some test coverage to make sure that it really isn't messed up.
>>> 
>>> -Filip
>>> 
>>> <GetProcessTarget.patch>
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130701/271c14ea/attachment.html>


More information about the llvm-commits mailing list