On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Mon, Mar 17, 2014 at 11:51 AM, Dragos Foianu <dragos.foi...@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.foi...@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...'
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
