[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