[Libguestfs] [PATCH nbdkit] cache, cow: Export block size constraints

[Laszlo Ersek]

On 02/21/22 15:32, Richard W.M. Jones wrote:
> 
> On Mon, Feb 21, 2022 at 03:19:23PM +0100, Laszlo Ersek wrote:
>> On 02/21/22 11:22, Richard W.M. Jones wrote:
>>> On Mon, Feb 21, 2022 at 10:22:04AM +0100, Laszlo Ersek wrote:
>>>>> +/* Block size constraints. */
>>>>> +static int
>>>>> +cache_block_size (nbdkit_next *next, void *handle,
>>>>> +                  uint32_t *minimum, uint32_t *preferred, uint32_t *maximum)
>>>>> +{
>>>>> +  if (next->block_size (next, minimum, preferred, maximum) == -1)
>>>>> +    return -1;
>>>>> +
>>>>> +  if (*minimum == 0) {         /* No constraints set by the plugin. */
>>>>> +    *minimum = 1;
>>>>> +    *preferred = blksize;
>>>>> +    *maximum = 0xffffffff;
>>>>> +  }
>>>>> +  else if (*maximum >= blksize) {
>>>>
>>>> Do we need braces here?
>>>>
>>>>> +    *preferred = MAX (*preferred, blksize);
>>>>> +  }
>>>
>>> I don't think we need them, but it might be clearer with them.
>>
>> Sorry, I'm still a bit confused whether braces around single statements
>> are *permitted* -- the rule even seems to vary across the various v2v
>> projects.
> 
> Oh I got the wrong end of the stick.  I thought you meant would the
> code be clearer like this:
> 
> +  if (*minimum == 0) {         /* No constraints set by the plugin. */
> +    *minimum = 1;
> +    *preferred = blksize;
> +    *maximum = 0xffffffff;
> +  }
> +  else {
> +    if (*maximum >= blksize)
> +      *preferred = MAX (*preferred, blksize);
> +  }
> 
> I think it would be clearer, since the else clause now refers to the
> separate case where the plugin *did* set constraints.  In fact I
> changed my local copy already.
> 
> But I think you meant should I have used braces around the single line
> statement (in the original code).
> 
>> My understanding has been that we forbid braces around single
>> statements, at least in some projects. So what's the rule?
> 
> There's not really a rule.  I tend to use whatever is clearer in the
> particular case.  ie. Trading off code density (being able to see more
> code on the page helps me) vs clarity (braces can be used to make it
> clearer what code is contained in a clause, but correct indentation
> does that as well).

Thanks! I've not met a project yet that wasn't very strict about braces
related to if/else. E.g. QEMU and edk2 require braces in all cases;
while Linux forbids braces around single statements (unless the proper
meaning of the code requires them).

I'm OK (of course!) with "whatever feels best on the spot".

Personally I've grown to "use braces everywhere", because (a) it's
easier to insert new stuff then, (b) my editor supports jumping from/to,
and highlighting, matching braces -- but that requires braces. :)

Thanks!
Laszlo