On Wed, Jan 23, 2002 at 06:28:06PM -0000, [EMAIL PROTECTED] wrote:
>...
> @@ -837,7 +844,8 @@
> return NULL;
> }
>
> - if (apr_file_info_get(&finfo, APR_FINFO_NORM, file) != APR_SUCCESS) {
> + rv = apr_file_info_get(&finfo, APR_FINFO_NORM, file);
Looking at the usage of 'finfo', this could be APR_FINFO_SIZE
>...
> @@ -1042,7 +1050,6 @@
> return err;
> }
>
> -#ifndef WIN32
> {
> dav_lock_discovery *ld;
> dav_lock_indirect *id;
> @@ -1071,7 +1078,6 @@
> return err;
> }
> }
> -#endif
Hmm. Debatable change. It could end up removing a lock record, then putting
it right back. I don't think it is a real problem, tho.
>...
> --- repos.c 14 Jan 2002 13:43:24 -0000 1.56
> +++ repos.c 23 Jan 2002 18:28:05 -0000 1.57
>...
> @@ -705,16 +733,12 @@
> dav_resource_private *ctx = resource->info;
> dav_resource_private *parent_ctx;
> dav_resource *parent_resource;
> + apr_status_t rv;
> char *dirpath;
>
> /* If given resource is root, then there is no parent */
> if (strcmp(resource->uri, "/") == 0 ||
> -#ifdef WIN32
> - (strlen(ctx->pathname) == 3 && ctx->pathname[1] == ':' &&
>ctx->pathname[2] == '/')
> -#else
> - strcmp(ctx->pathname, "/") == 0
> -#endif
> - ) {
> + ap_os_is_path_absolute(ctx->pool, ctx->pathname)) {
Bad and wrong.
The former tested for "is this the root". ap_os_is_path_absolute() is not
the same test. The end result is that none of the resources will have
parents, which is going to *completely* break the lock tests.
>...
> @@ -1171,13 +1195,21 @@
> else {
> const char *dirpath;
> apr_finfo_t finfo;
> + apr_status_t rv;
>
> /* destination does not exist, but the parent directory should,
> * so try it
> */
> dirpath = ap_make_dirstr_parent(dstinfo->pool, dstinfo->pathname);
> - if (apr_stat(&finfo, dirpath, APR_FINFO_NORM, dstinfo->pool) == 0
> - && finfo.device == srcinfo->finfo.device) {
> + /*
> + * XXX: If missing dev ... then what test?
> + * Really need a try and failover for those platforms.
> + *
> + */
> + rv = apr_stat(&finfo, dirpath, APR_FINFO_DEV, dstinfo->pool);
> + if ((rv == APR_SUCCESS || rv == APR_INCOMPLETE)
> + && (finfo.valid & srcinfo->finfo.valid & APR_FINFO_DEV)
> + && (finfo.device == srcinfo->finfo.device)) {
> can_rename = 1;
If the device doesn't exist or doesn't match, then it just does a copy.
>...
> @@ -1868,11 +1900,11 @@
> {
> const dav_liveprop_spec *info;
>
> -#ifndef WIN32
> - /* this property is not usable (writable) on the Win32 platform */
> + /* XXX this property is not usable (writable) on all platforms
> + * Need an abstract way to determine this.
> + */
> if (propid == DAV_PROPID_FS_executable && !resource->collection)
> return 1;
> -#endif
This is a bad change. Unless/until you get the query, then you should not be
removing this check. You're now advertising a property as being present, but
is ineffectual.
There are clients out there today which know and handle this custom
property. You're breaking them.
>...
> @@ -2036,10 +2068,11 @@
>
> void dav_fs_gather_propsets(apr_array_header_t *uris)
> {
> -#ifndef WIN32
> + /* XXX: Need an abstract way to determine this on a per-filesystem basis
> + * This may change by uri (!) (think OSX HFS + unix mounts).
> + */
> *(const char **)apr_array_push(uris) =
> "<http://apache.org/dav/propset/fs/1>";
> -#endif
> }
Same problem. The #ifndef should be restored unless/until you have the query
mechanism in place.
>...
> @@ -2077,16 +2110,8 @@
> what, phdr);
> (void) dav_fs_insert_prop(resource, DAV_PROPID_getetag,
> what, phdr);
> -
> -#ifndef WIN32
> - /*
> - ** Note: this property is not defined on the Win32 platform.
> - ** dav_fs_insert_prop() won't insert it, but we may as
> - ** well not even call it.
> - */
> (void) dav_fs_insert_prop(resource, DAV_PROPID_FS_executable,
> what, phdr);
> -#endif
And one more to put back.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
