Re: [PATCH v3] rust: alloc: fix dangling pointer in VecExt<T>::reserve()

[Danilo Krummrich <dakr@redhat.com>]

On 5/6/24 19:50, Miguel Ojeda wrote:
> On Mon, May 6, 2024 at 5:22 PM Danilo Krummrich <dakr@redhat.com> wrote:
>>
>> Is there any benefit using an if here, or is that your personal preference?
> 
> `if` is meant to test a boolean condition, which is what you are doing
> here, so the question is why would we want to use a `match` here -- we
> should first of all try to be consistent.
> 
> Now, I can understand the appeal of how the `match` looks like (e.g.
> less braces, less zig-zag indentation), and if this were a `if`-`else`
> chain with several cases and the wildcard `_` was not used, e.g.
> something like:
> 
>      ... = match n {
>          1 => ...
>          2 => ...
>          3 => ...
>      }
> 
> then I agree it would be clearly better.
> 
> Perhaps you are arguing for something like "let's always use `match`
> in cases where a value is returned, i.e. when used as an expression,
> and save `if` for the C-like cases". If so, would that mean then that
> we need to do:
> 
>      match c {
>          false => ...
>          true => ...
>      }
> 
> to be consistent?
> 
> Then there are other things like `if let`, the potential `is` operator
> and postfix `match` that should probably be considered.
> 

Thanks for your thoughts, but style wise I don't have a strong preference
for either of those. My question was honestly about whether there is any
other reason other than style.

However, I don't think using match is that unreasonable. There are cases
in the std library as well, e.g. [1]. But again, I'm fine with either of
those.

> Anyway, I am picking the fix as it is.

Please feel free to change the match statement to an if statement if that's
what you prefer.

- Danilo

[1] https://doc.rust-lang.org/src/alloc/string.rs.html#1358

> 
> Cheers,
> Miguel
>