Julian Foad <julianfoad@btopenworld.com> wrote on 02/09/2006 06:50:17 PM:
Thanks Julian (and Philip) for taking a look at this. A new log message
and patch is at the bottom which incorporates your suggestions.
> Do we already have the "configure" magic that sets this macro to
> true or false
> on every system?
No, AS400 and AS400_UTF8 are symbols IBM defines in their APR/Apache port.
Since the whole unix build process doesn't work on the iSeries we use a
custom build program that defines these two symbols. Your point is well
taken however since we don't define them to any specific value...
> If not (if it is set for AS400_UTF8 systems and unset
> elsewhere) then the directive should be "ifdef".
...so #if is out, I'll make sure #ifdef is used going forward.
> I see that the whole change is repeated verbatim in every "main.c"
> source file.
> I would very much like to see both the total amount of duplication and
the
> number of system-specific blocks added to each file kept as small
aspossible.
> Therefore I hope we can put this whole change inside a helper function
and
> reduce it to a single call, such as:
>
> #if AS400_UTF8
> SVN_INT_ERR (svn_cmdline_args_from_ebcdic (&argv, argv, pool));
> #endif
>
> to change argv before using it.
Ouch, I always thought redundant code was a good thing ;-) My revised
patch moves this functionality into your suggested function. But I
confess I'm a bit uneasy about how I modify argv. I pass &argv to
svn_cmdline_args_from_ebcdic(), convert the args to utf-8 in a separately
allocated char ** and then cast away argv's "constness" so I can point it
at the converted char **. Is avoiding const like this legit? (Somewhere
in the back of my head the ghostly voice of a CS TA is saying
"Woooo...don't cast away a const!" :-)
> Or, even better, don't we need to convert the arguments to UTF-8 on
every
> system? In that case,
>
> SVN_INT_ERR (svn_cmdline_args_to_utf8 (&argv, argv, pool));
>
> unconditionally on every system, and remove the later native-to-UTF-8
> processing that we presently do in dribs and drabs. I've sent a
> separate email
> about that, as people interested in that change might well not be
> reading this
> thread.
>
> If that thread doesn't go anywhere quickly, and you want to get
thischange in
> anyway, please could you write it as an svn_cmdline_args_from_ebcdic()
> function, in subversion/libsvn_subr/cmdline.c
> (subversion/include/svn_cmdline.h). Thanks. At least, that way, itwe
later
> do it that way on all systems, the change to callers will only be a
rename of
> the function and a removal of the "#if"s.
I'm a bit too busy this V5R4 stuff to dive into your first proposal at the
moment, sorry. I'd be glad to circle back to it once I'm through with
this series of OS400 patches though.
> Actually, that makes me wonder: if your change converts the
> arguments to UTF-8,
> do the "native-to-utf8" routines that are called later on know that
> they don't
> need to do anything? I suppose they do know this, because your compiler
> creates an environment in which "native" equals "UTF-8".
This touches upon my next patch which addresses string conversion; but all
conversions from native to UTF-8 and vice-versa do happen, in the sense
that apr_xlate_conv_buffer() is called. I can't say IBM's
apr_xlate_conv_buffer() "does anything" or not in these cases if that is
what you meant(?).
In hindsight I should have presented that next patch first, since this
(argv) patch is dependent on it. Briefly, this is because IBM uses ints
not strings as arguments to the apr_xlate_* functions. So a call like
"svn_utf_cstring_to_utf8_ex (&argv_utf8[i], (*argv)[i], "0",
SVN_UTF_ETOU_XLATE_HANDLE, pool)" in this patch won't work without the
upcoming patch which converts "0" to 0.
[[[
OS400/EBCDIC Port: Convert command-line arguments from EBCDIC to UTF-8.
This is the first of several patches to allow Subversion to run on under
IBM's
OS400 V5R4. Despite IBM's building of APR/Apache with UTF support in
V5R4,
command line arguments to main() are still encoded in EBCDIC.
* subversion/include/svn_cmdline.h
(svn_cmdline_args_from_ebcdic): New function declaration.
* subversion/libsvn_subr/cmdline.c
(SVN_UTF_ETOU_XLATE_HANDLE): New xlate key for EBCDIC (CCSID 0) to
UTF-8 (CCSID 1208) conversions.
(svn_cmdline_args_from_ebcdic): New function definition.
* subversion/svn/main.c
* subversion/svnadmin/main.c
* subversion/svndumpfilter/main.c
* subversion/svnlook/main.c
* subversion/svnserve/main.c
* subversion/svnsync/main.c
(main): Convert command line arguments from EBCDIC to UTF-8
with svn_cmdline_args_from_ebcdic().
]]]
Index: subversion/include/svn_cmdline.h
===================================================================
--- subversion/include/svn_cmdline.h (revision 18362)
+++ subversion/include/svn_cmdline.h (working copy)
@@ -247,6 +247,16 @@
void *cancel_baton,
apr_pool_t *pool);
+#ifdef AS400_UTF8
+/** Convert each of the @argc strings in @argv from ebcdic to utf-8.
+ * @a pool is used for all allocations.
+ */
+svn_error_t *
+svn_cmdline_args_from_ebcdic (int argc,
+ char * const * const *argv,
+ apr_pool_t *pool);
+#endif
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c (revision 18362)
+++ subversion/libsvn_subr/cmdline.c (working copy)
@@ -46,6 +46,10 @@
#define SVN_UTF_CONTOU_XLATE_HANDLE "svn-utf-contou-xlate-handle"
#define SVN_UTF_UTOCON_XLATE_HANDLE "svn-utf-utocon-xlate-handle"
+#ifdef AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn_utf-etou_xlate_handle"
+#endif
+
/* The stdin encoding. If null, it's the same as the native encoding. */
static const char *input_encoding = NULL;
@@ -446,3 +450,22 @@
return SVN_NO_ERROR;
}
+
+#ifdef AS400_UTF8
+svn_error_t *
+svn_cmdline_args_from_ebcdic (int argc,
+ char * const * const *argv,
+ apr_pool_t *pool)
+{
+ int i;
+ char **argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
+ for (i = 0; i < argc; i++)
+ {
+ SVN_ERR (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], (*argv)[i],
"0",
+ SVN_UTF_ETOU_XLATE_HANDLE,
+ pool));
+ }
+ *((char***)argv) = argv_utf8;
+ return SVN_NO_ERROR;
+}
+#endif
\ No newline at end of file
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 18362)
+++ subversion/svn/main.c (working copy)
@@ -830,6 +830,10 @@
return EXIT_FAILURE;
}
+#ifdef AS400_UTF8
+ SVN_INT_ERR (svn_cmdline_args_from_ebcdic (argc, &argv, pool));
+#endif
+
/* Else, parse options. */
apr_getopt_init (&os, pool, argc, argv);
os->interleave = 1;
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c (revision 18362)
+++ subversion/svnadmin/main.c (working copy)
@@ -1178,6 +1178,10 @@
opt_state.start_revision.kind = svn_opt_revision_unspecified;
opt_state.end_revision.kind = svn_opt_revision_unspecified;
+#ifdef AS400_UTF8
+ SVN_INT_ERR (svn_cmdline_args_from_ebcdic (argc, &argv, pool));
+#endif
+
/* Parse options. */
apr_getopt_init (&os, pool, argc, argv);
os->interleave = 1;
Index: subversion/svndumpfilter/main.c
===================================================================
--- subversion/svndumpfilter/main.c (revision 18362)
+++ subversion/svndumpfilter/main.c (working copy)
@@ -1121,6 +1121,10 @@
opt_state.start_revision.kind = svn_opt_revision_unspecified;
opt_state.end_revision.kind = svn_opt_revision_unspecified;
+#ifdef AS400_UTF8
+ SVN_INT_ERR (svn_cmdline_args_from_ebcdic (argc, &argv, pool));
+#endif
+
/* Parse options. */
apr_getopt_init (&os, pool, argc, argv);
os->interleave = 1;
Index: subversion/svnlook/main.c
===================================================================
--- subversion/svnlook/main.c (revision 18362)
+++ subversion/svnlook/main.c (working copy)
@@ -2026,6 +2026,10 @@
memset (&opt_state, 0, sizeof (opt_state));
opt_state.rev = SVN_INVALID_REVNUM;
+#ifdef AS400_UTF8
+ SVN_INT_ERR (svn_cmdline_args_from_ebcdic (argc, &argv, pool));
+#endif
+
/* Parse options. */
apr_getopt_init (&os, pool, argc, argv);
os->interleave = 1;
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c (revision 18362)
+++ subversion/svnserve/main.c (working copy)
@@ -313,6 +313,10 @@
return EXIT_FAILURE;
}
+#ifdef AS400_UTF8
+ SVN_INT_ERR (svn_cmdline_args_from_ebcdic (argc, &argv, pool));
+#endif
+
apr_getopt_init(&os, pool, argc, argv);
params.root = "/";
Index: subversion/svnsync/main.c
===================================================================
--- subversion/svnsync/main.c (revision 18362)
+++ subversion/svnsync/main.c (working copy)
@@ -1199,6 +1199,10 @@
svn_pool_destroy (pool);
return EXIT_FAILURE;
}
+
+#ifdef AS400_UTF8
+ SVN_INT_ERR (svn_cmdline_args_from_ebcdic (argc, &argv, pool));
+#endif
apr_getopt_init (&os, pool, argc, argv);
_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Feb 10 18:23:36 2006