Paul Burba wrote:
>
> [[[
> OS400/EBCDIC Port: Make svn_io_copy_file CCSID insensitive.
>
> This is one of several patches to allow Subversion to run on IBM's
> OS400 V5R4. It makes svn_io_copy_file() insensitive to the CCSIDs of
> the source and destination files, thereby preventing potential OS400
> conversion of the files' contents.
>
> * subversion/libsvn_subr/io.c
> (os400_file_copy): New function.
> (svn_io_copy_file): Replace call to apr_file_copy() with new
> function.
> ]]]
What do we want svn_io_copy_file() to do, in high-level terms? It seems we
want it to make an exact copy of a file, at least an exact copy of the
contents, but perhaps not the meta-data. But it's silly, isn't it, to preserve
the contents while not preserving the CCSID, because then the new file's CCSID
would be more likely to be wrong, so this function should copy the source
file's CCSID too.
I believe the apr_file_copy() was intended to do a byte-for-byte copy and thus
correctly copy any file, text or binary, even on systems that otherwise might
do translations on text files. However, it's broken on non-Unix systems.
There are two possible causes of unwanted modification during a copy.
(1) If the source and destination are opened in different modes (text vs.
binary) or, on OS400, have different CCSIDs. This should not occur because
this is a complete COPY operation in which the destination file is completely
under the function's control. The destination should be opened in the same
mode, and have the same CCSID as the source. If that's not easy to do with
APR, that's a bug in APR or needs to be done with lower-level system calls.
(2) If translating reads and writes are used, and the translation is not
exactly reversible, so read + write causes an unintended translation. This
should be avoided by opening the files in "binary" mode so that byte-for-byte
reads and writes are performed. That's the bug that Max Bowsher found for the
CygWin port of APR: it needs to read and write in binary mode. The same fix
should make the reads and writes insensitive to CCSIDs on OS400.
This should all be done within APR, of course. I can accept this patch on the
basis that it won't get into APR soon enough, but I would like some assurance
that one of you will make an effort to get it into APR.
To complete this patch, svn_io_copy_file() needs to say in its doc-string that
it does a "byte-for-byte" copy, as it is no longer just a wrapper around
apr_file_copy(). (Eventually, I hope apr_file_copy() will be fixed and
documented to do the same.)
> +#ifdef AS400
> +/* CCSID insensitive replacement for apr_file_copy() on OS400.
It's not simply a replacement, as it has an extra argument. Please either
remove or document the extra argument.
> + *
> + * (See comments for file_open() for more info on CCSIDs.)
> + *
> + * On OS400 apr_file_copy() attempts to convert the contents of the source
> + * file from its CCSID to the CCSID of the desination file. This may
("desination" -> "destination")
> + * corrupt the destination file's contents if the files' CCSIDs differ from
> + * each other and/or the system CCSID.
> + *
> + * This new function prevents this by forcing a binary copy. It is
> + * stripped down copy of the private function apr_file_transfer_contents in
> + * srclib/apr/file_io/unix/copy.c of version 2.0.54 of the Apache HTTP
> + * Server (http://httpd.apache.org/) excepting that APR_LARGEFILE is not
> + * used, from_path is always opened with APR_BINARY, and
> + * APR_FILE_SOURCE_PERMS is not supported.
> + */
> +static apr_status_t
> +os400_file_copy(const char *from_path,
> + const char *to_path,
> + apr_int32_t flags,
> + apr_fileperms_t perms,
> + apr_pool_t *pool)
> +#ifndef AS400
> apr_err = apr_file_copy(src_apr, dst_tmp_apr, APR_OS_DEFAULT, pool);
> +#else
> + /* Do a CCSID blind file copy on OS400. */
(No need for that comment. No space before paren in next line(s).)
> + apr_err = os400_file_copy (src_apr, dst_tmp_apr,
> + APR_WRITE | APR_CREATE | APR_TRUNCATE |
> + APR_BINARY, APR_OS_DEFAULT, pool);
> +#endif
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 7 15:59:51 2006