Re: [PATCH v4 6/6] fetch: delay user information post committing of transaction

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>>> +static void ref_update_display_info_free(struct ref_update_display_info *info)
>>>> +{
>>>> +	free((char *)info->summary);
>>>> +	free((char *)info->success_detail);
>>>> +	free((char *)info->fail_detail);
>>>> +	free((char *)info->remote);
>>>> +	free((char *)info->ref);
>>>> +}
>>>
>>> This answers "no" to my previous question.  These are not borrowed,
>>> but are owned by this structure.
>>>
>>
>> Yup, cannot be borrowed, since those go out of scope much earlier.
>
> And the reason why they are marked "const char *" which typically
> signals that they are borrowed is?  After all, that is where these
> casts inside free() comes from.
>
> There are two schools of thought.  One (which I originally was in)
> marks resources we own with "const", if these members will not
> change once we initialize them and we want to avoid accidentally
> muck with the contents of these pieces of memory during the course
> of the program.  Those of us in the school often have to cast away
> constness in their calls to free() like the above.
>

> But I saw many of our developers squarely fall into the other camp,
> where they always use a non-const pointer to point at the resource
> the structure owns.
>
> The latter school of thought opens us up to bugs caused by mistaken
> code that modifies these memory regions that those of us in the
> former school would use "const" to avoid, but it makes it easier to
> reason about memory ownership models by signalling if the enclosing
> structure owns or borrows the resources.
>
> I'd say the latter school are majority of our developer base, and a
> lot of existing structures follow that rule.  I was hinting that we
> may want to follow suit in this new structure.
>
> Thanks.