You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "B. Wells" <b_...@appleisp.net> on 2006/07/24 03:27:39 UTC

[PATCH] Workaround for Mac OS X 10.4 SMB bug

For this workaround, I followed the same pattern as the other Mac OS X
hack in the source file. A simpler patch would have been to prevent zero
byte write operations for all newly created files. But it may be that
such a write operation is needed on some file systems to properly create
the new file. Thus this patch compiles specificly for Mac OS X and only
alters functionality when creating files on SMB shares.

Of course, this patch is really useful when APR is also patched to
support file locking on SMB shares. I wrote such a patch several months
ago and posted it on my website.

http://homepage.mac.com/brianwells/.Public/svn_darwin_flock_patch

I should clean it up and post it to the dev@apr.apache.org mailing list
to see if they are interested in it. Or perhaps even better would be to
redo it as a patch for svn_io_file_lock2 so that the patches stay with
Subversion.

Any suggestions?

– Brian Wells
b_wells@appleisp.net

[[[
Workaround for Mac OS X 10.4 SMB bug that prevents creation of empty  
files.

This is a workaround for a Mac OS X 10.4 bug that results in an I/O  
error
when an empty file is created by Subversion on an SMB share. This  
typically
happens when working with an FSFS-backed repository stored on a  
Windows file
server.

* subversion/libsvn_subr/io.c
   (svn_io_file_create): Prevent write operations of zero bytes for  
Mac OS X
   when the file being created is on an SMB share.

]]]

Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c	(.../vendor/subversion/current)	 
(revision 31)
+++ subversion/libsvn_subr/io.c	(.../trunk/subversion)	(revision 31)
@@ -770,6 +770,29 @@
    return SVN_NO_ERROR;
}
+
+/*
+ Some of the most recent releases of Mac OS X 10.4 (10.4.7 for sure)  
have a
+ bug that results in an I/O error when an attempt is made to perform  
a write
+ operation of zero bytes to a file on a mounted SMB share. This is a  
big
+ problem when Subversion calls svn_io_file_create() to create empty  
files.
+
+ This workaround will avoid write operations of zero bytes if the  
file being
+ created is on an SMB share.
+ */
+#if defined(__APPLE__) && defined(__MACH__)
+#include <sys/param.h>
+#include <sys/mount.h>
+#define MACOSX_SMBWRITE_HACK 
(file,contents)                                   \
+struct statfs  
sfb;                                                            \
+if (strlen(contents) !=  
0                                                     \
+    || statfs(file, &sfb) ==  
-1                                               \
+    || apr_strnatcmp(sfb.f_fstypename,"smbfs") != 0)
+#else
+#define MACOSX_SMBWRITE_HACK(file, contents) do {} while (0);
+#endif
+
+
svn_error_t *svn_io_file_create (const char *file,
                                   const char *contents,
                                   apr_pool_t *pool)
@@ -781,6 +804,7 @@
                               (APR_WRITE | APR_CREATE | APR_EXCL),
                               APR_OS_DEFAULT,
                               pool));
+  MACOSX_SMBWRITE_HACK(file,contents)
    SVN_ERR (svn_io_file_write_full (f, contents, strlen (contents),
                                     &written, pool));
    SVN_ERR (svn_io_file_close (f, pool));

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH] Workaround for Mac OS X 10.4 SMB bug

Posted by Julian Foad <ju...@btopenworld.com>.
Thanks for the patch submission.  However, I have several concerns about it.

B. Wells wrote:
> For this workaround, I followed the same pattern as the other Mac OS X
> hack in the source file.

Where is that?  I can't find one.

Which version of this source file are you looking at?  The trunk version has 
svn_io_file_create() at line 960, not around 770.


> A simpler patch would have been to prevent zero
> byte write operations for all newly created files. But it may be that
> such a write operation is needed on some file systems to properly create
> the new file. Thus this patch compiles specificly for Mac OS X and only
> alters functionality when creating files on SMB shares.

I don't think there is any need to perform the zero-byte write on any systems, 
but I am not sure.


> Of course, this patch is really useful when APR is also patched to
> support file locking on SMB shares. I wrote such a patch several months
> ago and posted it on my website.
> 
> http://homepage.mac.com/brianwells/.Public/svn_darwin_flock_patch
> 
> I should clean it up and post it to the dev@apr.apache.org mailing list
> to see if they are interested in it. Or perhaps even better would be to
> redo it as a patch for svn_io_file_lock2 so that the patches stay with
> Subversion.

If it's doing something that a portability layer should be doing, please post 
it to the APR development list.

> Index: subversion/libsvn_subr/io.c
> ===================================================================
> --- subversion/libsvn_subr/io.c    (.../vendor/subversion/current)    (revision 31)
> +++ subversion/libsvn_subr/io.c    (.../trunk/subversion)    (revision 31)
> @@ -770,6 +770,29 @@
>    return SVN_NO_ERROR;
> }

(One of the three lines of context seems to be missing from the patch here - 
maybe a problem with your mail program.  If this always happens, consider 
attaching the patch (with a text MIME type) instead of pasting it.)

> +
> +/*
> + Some of the most recent releases of Mac OS X 10.4 (10.4.7 for sure) have a
> + bug that results in an I/O error when an attempt is made to perform a write
> + operation of zero bytes to a file on a mounted SMB share. This is a big
> + problem when Subversion calls svn_io_file_create() to create empty files.
> +
> + This workaround will avoid write operations of zero bytes if the file being
> + created is on an SMB share.
> + */
> +#if defined(__APPLE__) && defined(__MACH__)
> +#include <sys/param.h>
> +#include <sys/mount.h>
> +#define MACOSX_SMBWRITE_HACK (file,contents)           \
> +struct statfs  sfb;                                    \
> +if (strlen(contents) !=  0                             \
> +    || statfs(file, &sfb) ==  -1                       \
> +    || apr_strnatcmp(sfb.f_fstypename,"smbfs") != 0)

Have you tested this?  The space before the macro's parentheses should mean it 
defines as a macro without an argument list, and thus I'd expect compilation to 
fail.

The "struct" definition will appear after statements, which is not valid ANSI 
C'89 even if your compiler accepts it.

Repeating the "strlen" operation here is inefficient.

> +#else
> +#define MACOSX_SMBWRITE_HACK(file, contents) do {} while (0);

This definition might as well be blank: there's no need for it to evaluate to a 
statement when the other version doesn't.

> +#endif
> +
> +
> svn_error_t *svn_io_file_create (const char *file,
>                                   const char *contents,
>                                   apr_pool_t *pool)
> @@ -781,6 +804,7 @@
>                               (APR_WRITE | APR_CREATE | APR_EXCL),
>                               APR_OS_DEFAULT,
>                               pool));
> +  MACOSX_SMBWRITE_HACK(file,contents)
>    SVN_ERR (svn_io_file_write_full (f, contents, strlen (contents),
>                                     &written, pool));
>    SVN_ERR (svn_io_file_close (f, pool));

Is this the only place where we might perform a zero-byte write?  Wouldn't it 
be better to put this hack in svn_io_file_write_full() and svn_io_file_write()? 
  Or, better still, in APR?

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org