Re: [PATCH 04/10] packed_delete_refs(): implement method

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Aug 30, 2017, 03:00 UTC
Message-ID: <5237dffe-fcb4-37b9-2645-3f0f0dc5bd19@alum.mit.edu>
In-Reply-To: <20170829180732.GA131745@google.com>

On 08/29/2017 08:07 PM, Brandon Williams wrote:

Show 22 quoted lines

> On 08/29, Michael Haggerty wrote:
>> [...]
>> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
>> index d19b3bfba5..83a088118f 100644
>> --- a/refs/packed-backend.c
>> +++ b/refs/packed-backend.c
>> @@ -1082,7 +1082,50 @@ static int packed_initial_transaction_commit(struct ref_store *ref_store,
>>  static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
>>  			     struct string_list *refnames, unsigned int flags)
>>  {
>> -	die("BUG: not implemented yet");
>> +	struct packed_ref_store *refs =
>> +		packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");
>> +	struct strbuf err = STRBUF_INIT;
>> +	struct ref_transaction *transaction;
>> +	struct string_list_item *item;
>> +	int ret;
>> +
>> +	(void)refs; /* We need the check above, but don't use the variable */
> 
> Can't say I've seen a line like this before, Is the intent to just mark
> that this variable won't be used like you mention in the comment?

The `(void)refs;` part tells the compiler not to complain about a variable that is defined but never used.

So why define the variable in the first place? I could instead do

    (void)packed_downcast(ref_store, REF_STORE_WRITE, "delete_refs");

My reason (which people might disagree with) is that I like to keep this boilerplate consistent from method to method. To switch to the second variant, not only would the line look different, but it would also have to be moved down several lines, past the other variables' declarations, to avoid the dreaded "ISO C90 forbids mixed declarations and code".

I'd prefer to be able to mix declarations and code, but the project's style prohibits it.

Michael