Re: [PATCHv2] branch.c: simplify chain of if statements

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

 



On Wed, Mar 19, 2014 at 7:12 PM, Dragos Foianu <dragos.foianu@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine <at> sunshineco.com> writes:
>> On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine <sunshine <at>
> sunshineco.com> wrote:
>>
>> One other observation: You have a one-off error in your out-of-bounds
>> check. It should be 'index >= sizeof...'
>
> Well this is embarrassing.

It's a good illustration of the value of the review process. It's easy
to overlook omissions and problems in our one's work because one reads
it with the bias of knowing what it's _supposed_ to say. Reviewers
(hopefully) don't have such bias: they read the code afresh.

> Thank you again for the feedback. It's incredibily helpful and I learned a
> lot from submitting these patches. Making the code simple is harder than it
> appears at first sight.
>
> I'm not sure it's worth pursuing the table approach further, especially
> since a solution has already been accepted and merged into the codebase.

Agreed.

> In this case, is it okay to try another microproject? I was thinking about
> trying #17 (finding bugs/inefficiencies in builtin/apply.c), but I've
> already had my one micro project.

According to the description for #17, there are plenty of opportunities, so...

> All the best,
> Dragos
--
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]