Am 21.09.19 um 16:21 schrieb SZEDER Gábor:
> On Sat, Sep 21, 2019 at 02:37:05PM +0200, René Scharfe wrote:
>> Am 20.09.19 um 20:13 schrieb SZEDER Gábor:
>>>>> @@ -280,12 +269,16 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
>>>>> if (o && o->type == OBJ_COMMIT) {
>>>>> struct commit *commit = (struct commit *)o;
>>>>> int from_tag = starts_with(path, "refs/tags/");
>>>>> + const char *tip_name;
>>>>
>>>> This should not be const because you allocate the buffer it points to
>>>> right here in the function, in each execution path.
>>>
>>> Marking it as const indicates that this function doesn't modify the
>>> buffer where the pointer points at.
>>
>> Right, and that's at odds with this code:
>>
>>>>> + if (deref)
>>>>> + tip_name = xstrfmt("%s^0", path);
>>>>> + else
>>>>> + tip_name = xstrdup(path);
>>
>> ... which allocates said memory and writes a string to it.
>
> ... before assigning it to the const pointer.
>
Sure, you can cast anything to anything else, and slapping on a const
qualifier is even allowed to be done implicitly for pointers to objects
(but not for pointers to pointers). Removing it later (e.g. for
free(3)) is a warning sign; such sites need to be checked manually, as
the compiler won't do it.
The declaration says we don't modify the buffer, but then we actually
create it, which is as big a modification as can be. That's a bit
misleading. Is protection against accidental updates worth the
misdirection, and where would they come from? Usually code without
such tricks is easier to read and maintain.
René
