Re: [PATCHv2] branch.c: simplify chain of if statements
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 18, 2014, 23:50 UTC
- Message-ID
- <CAPig+cQKHQFNBob18g9UmZuE_mOpF3UMCBPfSKJYEYQpk1Z_tw@mail.gmail.com>
- In-Reply-To
- <CAPig+cS9QApn1T3-R8n+W+1ee9FbNftsmhrr90SJKs+gqzvC5A@mail.gmail.com>
On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 28 quoted lines
> On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu <dragos.foianu@gmail.com> 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@gmail.com>
>> ---
>> 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...'
Show 10 quoted lines
> 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