Mailing List Archive

Re: svn commit: r1907608 - in /httpd/httpd/trunk: CMakeLists.txt modules/dav/main/NWGNUmakefile modules/dav/main/config5.m4 modules/dav/main/mod_dav.c modules/dav/main/mod_dav.dsp modules/dav/main/mod_dav.h modules/dav/main/ms_wdv.c modules/dav/main/util.
Le 13/02/2023 à 17:48, manu@apache.org a écrit :
> Author: manu
> Date: Mon Feb 13 16:48:35 2023
> New Revision: 1907608
>
> URL: http://svn.apache.org/viewvc?rev=1907608&view=rev
> Log:
> Add MS-WDV support
>
> MS-WDV specification:
> https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-wdv
>
> The changes introduces the DAVMSext directive, which is used to
> enable MS-WDV: DAVMSext +WDV
>
> dav_get_timeout_string() is introduced as a variant of dav_get_timeout().
> The former parses a string, the later parse the Timeout HTTP header.
>
>
> Added:
> httpd/httpd/trunk/modules/dav/main/ms_wdv.c
> Modified:
> httpd/httpd/trunk/CMakeLists.txt
> httpd/httpd/trunk/modules/dav/main/NWGNUmakefile
> httpd/httpd/trunk/modules/dav/main/config5.m4
> httpd/httpd/trunk/modules/dav/main/mod_dav.c
> httpd/httpd/trunk/modules/dav/main/mod_dav.dsp
> httpd/httpd/trunk/modules/dav/main/mod_dav.h
> httpd/httpd/trunk/modules/dav/main/util.c
>

[...]

> /*
> + * Command handler for the DAVmsExt directive, which is RAW
> + */
> +static const char *dav_cmd_davmsext(cmd_parms *cmd, void *config, const char *w)
> +{
> + dav_dir_conf *conf = (dav_dir_conf *)config;
> +
> + if (!ap_cstr_casecmp(w, "None"))
> + conf->msext_opts = DAV_MSEXT_NONE;
> + else if (!ap_cstr_casecmp(w, "Off"))
> + conf->msext_opts = DAV_MSEXT_NONE;

Hi, I think that:
if (!ap_cstr_casecmp(w, "None") ||
!ap_cstr_casecmp(w, "Off"))
conf->msext_opts = DAV_MSEXT_NONE;

would be more readable. It is easier to see that both are the same.
But it is mostly a matter of taste :).

(same below when setting DAV_MSEXT_ALL)

> + else if (!ap_cstr_casecmp(w, "+WDV"))
> + conf->msext_opts |= DAV_MSEXT_WDV;
> + else if (!ap_cstr_casecmp(w, "WDV"))
> + conf->msext_opts |= DAV_MSEXT_WDV;

is |= expected here?

> + else if (!ap_cstr_casecmp(w, "-WDV"))
> + conf->msext_opts &= ~DAV_MSEXT_WDV;
> + else if (!ap_cstr_casecmp(w, "All"))
> + conf->msext_opts = DAV_MSEXT_ALL;
> + else if (!ap_cstr_casecmp(w, "On"))
> + conf->msext_opts = DAV_MSEXT_ALL;
> + else
> + return "DAVMSext values can be None | [+|-]WDV | All";
> +
> + return NULL;
> +}
> +