| Message ID | 20230406213909.22717-3-stokito@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | [1/3] uclient-fetch: Make request_types sorted to optimize search | expand |
On Fri, Apr 07, 2023 at 12:39:09AM +0300, Sergey Ponomarev wrote: > The WebDAV is an extension for HTTP for shared folders. > In order to make wget working with it we have to declare the missing constants with methods. > They don't take part in a logic except of OPTIONS methods that can't have a body. > > Signed-off-by: Sergey Ponomarev <stokito@gmail.com> Please examine: https://openwrt.org/submitting-patches#submission_guidelines The commit message needs line-wrapping. > @@ -292,7 +308,7 @@ static void uclient_http_process_headers(struct uclient_http *uh) > > static bool uclient_request_supports_body(enum request_type req_type) > { > - return !(req_type == REQ_GET || req_type == REQ_HEAD); > + return !(req_type == REQ_GET || req_type == REQ_HEAD || req_type == REQ_OPTIONS); > } > > static int Here we have the problem mentioned in the previous message. That line is going to keep growing, and growing, and growing, and growing. Whereas if you merely invert the cases on the switch: case REQ_GET: case REQ_HEAD: case REQ_OPTIONS: return false; default: return true; The rest seems reasonable. Again, note I am *not* a maintainer, so this is *my* opinion and I do not have any authority (besides experience).
Thank you Ellitott for the review, I agree with all your points. I just made those changes for myself to support WebDAV methods but generally speaking this is not an important feature for anyone else. So let's reject the patch. The main problem is that any extended http method needs to be pre-declared inside of the uclient and ideally this should be changed to be more flexible. But this may break backward compatibility. On Sat, Apr 8, 2023 at 1:00 AM Elliott Mitchell <ehem+openwrt@m5p.com> wrote: > > On Fri, Apr 07, 2023 at 12:39:09AM +0300, Sergey Ponomarev wrote: > > The WebDAV is an extension for HTTP for shared folders. > > In order to make wget working with it we have to declare the missing constants with methods. > > They don't take part in a logic except of OPTIONS methods that can't have a body. > > > > Signed-off-by: Sergey Ponomarev <stokito@gmail.com> > > Please examine: > https://openwrt.org/submitting-patches#submission_guidelines > > The commit message needs line-wrapping. > > > @@ -292,7 +308,7 @@ static void uclient_http_process_headers(struct uclient_http *uh) > > > > static bool uclient_request_supports_body(enum request_type req_type) > > { > > - return !(req_type == REQ_GET || req_type == REQ_HEAD); > > + return !(req_type == REQ_GET || req_type == REQ_HEAD || req_type == REQ_OPTIONS); > > } > > > > static int > > Here we have the problem mentioned in the previous message. That line > is going to keep growing, and growing, and growing, and growing. Whereas > if you merely invert the cases on the switch: > > case REQ_GET: > case REQ_HEAD: > case REQ_OPTIONS: > return false; > > default: > return true; > > The rest seems reasonable. Again, note I am *not* a maintainer, so this > is *my* opinion and I do not have any authority (besides experience). > > > -- > (\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/) > \BS ( | ehem+sigmsg@m5p.com PGP 87145445 | ) / > \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ > 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445 > >
diff --git a/uclient-http.c b/uclient-http.c index 02bb7c2..b4e6d70 100644 --- a/uclient-http.c +++ b/uclient-http.c @@ -40,11 +40,19 @@ enum auth_type { }; enum request_type { + REQ_COPY, REQ_DELETE, REQ_GET, REQ_HEAD, + REQ_LOCK, + REQ_MKCOL, + REQ_MOVE, + REQ_OPTIONS, REQ_POST, + REQ_PROPFIND, + REQ_PROPPATCH, REQ_PUT, + REQ_UNLOCK, __REQ_MAX }; @@ -59,11 +67,19 @@ enum http_state { // Must be sorted aplhabeticaly static const char * const request_types[__REQ_MAX] = { + [REQ_COPY] = "COPY", [REQ_DELETE] = "DELETE", [REQ_GET] = "GET", [REQ_HEAD] = "HEAD", + [REQ_LOCK] = "LOCK", + [REQ_MKCOL] = "MKCOL", + [REQ_MOVE] = "MOVE", + [REQ_OPTIONS] = "OPTIONS", [REQ_POST] = "POST", + [REQ_PROPFIND] = "PROPFIND", + [REQ_PROPPATCH] = "PROPPATCH", [REQ_PUT] = "PUT", + [REQ_UNLOCK] = "UNLOCK", }; struct uclient_http { @@ -292,7 +308,7 @@ static void uclient_http_process_headers(struct uclient_http *uh) static bool uclient_request_supports_body(enum request_type req_type) { - return !(req_type == REQ_GET || req_type == REQ_HEAD); + return !(req_type == REQ_GET || req_type == REQ_HEAD || req_type == REQ_OPTIONS); } static int
The WebDAV is an extension for HTTP for shared folders. In order to make wget working with it we have to declare the missing constants with methods. They don't take part in a logic except of OPTIONS methods that can't have a body. Signed-off-by: Sergey Ponomarev <stokito@gmail.com> --- uclient-http.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)