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 <stok...@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+sig...@m5p.com PGP 87145445 | ) / \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445 _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel