[PATCH] Refactoring of the bio_bucket_ctrl() function

[Denis Kovalchuk <de...@visualsvn.com.INVALID>]

Hi!

I would like to propose a refactoring of the bio_bucket_ctrl() function in the
ssl_bucket.c file:

- Return values from case statements and get rid of the ret variable. The
control flow is more explicit and clear.

- Move the default statement to the end of the switch. It's more common for
switch statements.

What do you think?

Kind Regards,
Denis Kovalchuk

Re: [PATCH] Refactoring of the bio_bucket_ctrl() function

[Denis Kovalchuk <de...@visualsvn.com.INVALID>]

> > - Return values from case statements and get rid of the ret variable. The
> > control flow is more explicit and clear.
> >
> > I personally prefer the "we return an error by default unless we
> explicitly addressed the situation" attitude. So for me this part of your
> patch doesn't provide an improvement.
>
>
> > - Move the default statement to the end of the switch. It's more common for
> > switch statements.
> >
> > Yeah, the default case at the bottom is indeed more common. If you are
> working on bigger changes in the ssl code you can include this modification
> AFAIC.

Yes, this patch was intended as ground work for a couple of related changes.

One problem with the "long ret = 1;" pattern is that the return values are
different for different ctrls [1]. So "1" may be a reasonable default for some
of them, but entirely unreasonable for others.

I think that I will try to include the necessary changes as a part of the "real"
patch, so that they could be viewed in its context.

[1] https://www.openssl.org/docs/manmaster/man3/BIO_ctrl.html#RETURN-VALUES

Kind Regards,
Denis Kovalchuk

Re: [PATCH] Refactoring of the bio_bucket_ctrl() function

[Lieven Govaerts <lg...@apache.org>]

Hi Denis,

On Fri, 17 Jun 2022 at 14:59, Denis Kovalchuk
<de...@visualsvn.com.invalid> wrote:

> Hi!
>
> I would like to propose a refactoring of the bio_bucket_ctrl() function in
> the
> ssl_bucket.c file:
>
> - Return values from case statements and get rid of the ret variable. The
> control flow is more explicit and clear.
>
> I personally prefer the "we return an error by default unless we
explicitly addressed the situation" attitude. So for me this part of your
patch doesn't provide an improvement.


> - Move the default statement to the end of the switch. It's more common for
> switch statements.
>
> Yeah, the default case at the bottom is indeed more common. If you are
working on bigger changes in the ssl code you can include this modification
AFAIC.


> What do you think?
>
> Kind Regards,
> Denis Kovalchuk
>

regards,
Lieven