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

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

 



On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu <dragos.foianu@xxxxxxxxx> wrote:
>> This patch uses a table to store the different messages that can
>> be emitted by the verbose install_branch_config function. It
>> computes an index based on the three flags and prints the message
>> located at the specific index in the table of messages. If the
>> index somehow is not within the table, we have a bug.
>>
>> Signed-off-by: Dragos Foianu <dragos.foianu@xxxxxxxxx>
>> ---
>>  branch.c | 44 +++++++++++++++++++++++++-------------------
>>  1 file changed, 25 insertions(+), 19 deletions(-)
>>
>> diff --git a/branch.c b/branch.c
>> index 723a36b..95645d5 100644
>> --- a/branch.c
>> +++ b/branch.c
>> @@ -54,6 +54,18 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
>> +       int index = 0;
>> +       if (origin)
>> +               index += 1;
>> +       if (remote_is_branch)
>> +               index += 2;
>> +       if (rebasing)
>> +               index += 4;
>> +
>> +               if (index < 0 || index > sizeof(messages) / sizeof(*messages))
>>                         die("BUG: impossible combination of %d and %p",
>>                             remote_is_branch, origin);

One other observation: You have a one-off error in your out-of-bounds
check. It should be 'index >= sizeof...'

> You can use ARRAY_SIZE() in place of sizeof(...)/sizeof(...).
>
> Since an out-of-bound index would be a programmer bug, it would
> probably be more appropriate to use an assert(), just after 'index' is
> computed, rather than if+die(). The original code used die() because
> it couldn't detect the error until the end of the if-chain.
>
>>         }
>> --
>> 1.8.3.2
--
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]