[PATCH 2/3] uclient-fetch: Simplify uclient_request_supports_body()

[Elliott Mitchell]

On Fri, Apr 07, 2023 at 12:39:08AM +0300, Sergey Ponomarev wrote:
> Only GET and HEAD shouldn't have a body
> 
> Signed-off-by: Sergey Ponomarev <stokito at gmail.com>

Still kind of on the short side.  Also doesn't exactly describe what the
patch is doing.

> @@ -292,14 +292,7 @@ static void uclient_http_process_headers(struct uclient_http *uh)
>  
>  static bool uclient_request_supports_body(enum request_type req_type)
>  {
> -	switch (req_type) {
> -	case REQ_POST:
> -	case REQ_PUT:
> -	case REQ_DELETE:
> -		return true;
> -	default:
> -		return false;
> -	}
> +	return !(req_type == REQ_GET || req_type == REQ_HEAD);
>  }
>  
>  static int

Rather than one thing being done here, I see two distinct things being
done.

First, you're inverting the default logic.  You would get the same result
by having `case REQ_GET:`, `case REQ_HEAD:`, `return false;`.  Then
having the default `return true;`.
This seems reasonable to do, but isn't really any simpler.

Second, you're changing from a `switch` construct to a conditional
expression.  Since this is effectively a list of values, this seems
rather questionable on its own.  There is a reduction of 7 lines, but
you're substituting 7 short lines for one rather long one.
Notably this is less maintainable since that line keeps growing for each
and every case you add.

The first may go through, the second seems very dubious.

(note, I am *not* an OpenWRT maintainer, a maintainer could have a
different opinion)


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg at m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445