Re: [PATCH 1/1] general style: replaces memcmp() with starts_with()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:

>>> A patch of this nature doesn't require much more description than
>>> stating what it does ("replace memcmp() with starts_with()") and why
>>> ("improve code clarity"). The following rewrite might be sufficient:
>>>
>>>     Subject: replace memcmp() with starts_with()
>>>
>>>     starts_with() indicates the intention of the check more clearly
>>>     than memcmp().
>>
>> This is more concise; thank you. I will adapt this as the message for
>> the next version of this patch. Would it be wise to mention magic
>> numbers, as the discussion surrounding the rationale of this patch,
>> especially with Junio and Michael, has centered around that?
>
> I was thinking of mentioning magic numbers in the example, but decided
> that their removal was a natural and obvious consequence of the
> improvement to code clarity, so it wasn't strictly necessary to talk
> about it. On the other hand, it is a good secondary justification,
> thus it should be perfectly acceptable to mention it. If I was writing
> the commit message, I might start by saying "As an additional
> benefit..." and then talk a bit about magic number retirement.

I think your subject is good (as you said, it makes it clear that it
is a project-wide clean-up by not mentioning any specific area), but
"indicates the intention of the check more clearly" would not tell
new readers who are unfamiliar with the helper API what "intention"
it is talking about very much, so perhaps

	Subject: use starts_with() instead of !memcmp()

	When checking if a string begins with a constant string,
	using starts_with() is less error prone than calling
	!memcmp() with an explicit byte count.

or something?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]