svn commit: r327699 - head/sys/sys

[Pedro Giffuni]

Hello;

On 08/01/2018 15:27, Conrad Meyer wrote:
> I'm (again, atypically) with rgrimes here.  Revert has a specific
> meaning and shouldn't be used like this.  Making a commit with the
> subject "Revert rXXX" that does more than just "svn revert rXXX" makes
> life harder for developers looking at code history after you.  Making
> two separate commits for two different changes (revert, then add the
> annotation) is not burdensome.
>
> Best,
> Conrad

Yeah, I understand where that comes from and I will take it into account 
for future commits, but I think it should be *documented* and not assume 
that everybody  thinks that is the way version control is supposed to be 
used.

Pedro.

> On Mon, Jan 8, 2018 at 12:18 PM, Pedro Giffuni <pfg at freebsd.org> wrote:
>> On 08/01/2018 11:09, Rodney W. Grimes wrote:
>>>> Author: pfg
>>>> Date: Mon Jan  8 15:54:29 2018
>>>> New Revision: 327699
>>>> URL: https://svnweb.freebsd.org/changeset/base/327699
>>>>
>>>> Log:
>>>>     Revert r327697:
>>>>     malloc(9): drop the __result_use_check attribute for the kernel
>>>> allocator.
>>>>        My bad: __result_use_check just checks the for the general and we
>>>> always
>>>>     want to make sure allocated memory is used, not only checked for
>>>> nullness.
>>>>        Add it to reallocf since that was missing.
>>> Please try not to combine a revert with an add, it makes it messy
>>> to try and figure out things in the future when only the svn log
>>> is being used to analyze stuff as digging in mail archives becomes
>>> painful.
>>>
>>> Revert, then commit the add standalone, is the better sequence in
>>> this type of situation.
>>
>> Not that any of this is defined in our committers guide but IMHO "svn
>> revert" is just a tool, pretty much as "svn move" and "svn copy". The idea
>> is to make a committers' life easier: making two commits for such a simple
>> change is not "easier".
>>
>> In this case the change is rather consistent: I added __result_use_check to
>> the three functions that needed it.
>> The commit log is not only clear on why the revert happened but also
>> explains the additional one line change.
>>
>> When I MFC it, I will merge both changes for repository consistency but the
>> log will basically mention that I am adding __result_use_check to
>> reallocf().
>>
>>
>> Pedro.
>>
>>>> Modified:
>>>>     head/sys/sys/malloc.h
>>>>
>>>> Modified: head/sys/sys/malloc.h
>>>>
>>>> ==============================================================================
>>>> --- head/sys/sys/malloc.h       Mon Jan  8 15:41:49 2018        (r327698)
>>>> +++ head/sys/sys/malloc.h       Mon Jan  8 15:54:29 2018        (r327699)
>>>> @@ -176,7 +176,7 @@ void        *contigmalloc(unsigned long size, struct
>>>> malloc_t
>>>>              __alloc_size(1) __alloc_align(6);
>>>>    void  free(void *addr, struct malloc_type *type);
>>>>    void  *malloc(unsigned long size, struct malloc_type *type, int flags)
>>>> -           __malloc_like __alloc_size(1);
>>>> +           __malloc_like __result_use_check __alloc_size(1);
>>>>    void  *mallocarray(size_t nmemb, size_t size, struct malloc_type *type,
>>>>              int flags) __malloc_like __result_use_check
>>>>              __alloc_size(1) __alloc_size(2);
>>>> @@ -187,9 +187,9 @@ void        malloc_type_freed(struct malloc_type
>>>> *type, unsig
>>>>    void  malloc_type_list(malloc_type_list_func_t *, void *);
>>>>    void  malloc_uninit(void *);
>>>>    void  *realloc(void *addr, unsigned long size, struct malloc_type
>>>> *type,
>>>> -           int flags) __alloc_size(2);
>>>> +           int flags) __result_use_check __alloc_size(2);
>>>>    void  *reallocf(void *addr, unsigned long size, struct malloc_type
>>>> *type,
>>>> -           int flags) __alloc_size(2);
>>>> +           int flags) __result_use_check __alloc_size(2);
>>>>      struct malloc_type *malloc_desc2type(const char *desc);
>>>>    #endif /* _KERNEL */
>>>>
>>>>
>> Pedro.
>>