You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pa...@softlanding.com> on 2006/02/09 21:40:19 UTC

[PATCH] #1 Update on port to OS400/EBCDIC

Hello All,

This is a follow-up to Mark's message yesterday, see 
http://svn.haxx.se/dev/archive-2006-02/0519.shtml

This is the first of several (probably 5 - 10) patch submissions that will 
allow subversion to run on the IBM iSeries under OS400 V5R4.  I'll try to 
keep each patch relatively small and focused on one problem.

Ok then, here is patch #1.  It addresses the problem that argv arguments 
to main() are encoded in ebcdic.  If anyone out there has some time to 
review it we'd appreciate it.

Thanks,

Paul B.

================================================================================

[[[
* subversion/svn/main.c:
* subversion/svnadmin/main.c 
* subversion/svndumpfilter/main.c
* subversion/svnlook/main.c
* subversion/svnserve/main.c
* subversion/svnsync/main.c
* subversion/svnversion/main.c
   (SVN_UTF_ETOU_XLATE_HANDLE): Xlate key for ebcdic (CCSID 0) to utf-8
    conversions.
   (main): Convert incoming argv strings to utf-8.  Note that 
    svn_utf_cstring_to_utf8_ex() is used instead of 
svn_utf_cstring_to_utf8
    because V5R4 thinks native strings are in utf-8.
]]]

Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c       (revision 18362)
+++ subversion/svn/main.c       (working copy)
@@ -48,6 +48,9 @@
 
 #include "svn_private_config.h"
 
+#if AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
+#endif
 
 /*** Option Processing ***/
 
@@ -780,6 +783,11 @@
   svn_cl__cmd_baton_t command_baton;
   svn_auth_baton_t *ab;
   svn_config_t *cfg;
+#if AS400_UTF8
+  /* Even when compiled with UTF support in V5R4, argv is still
+   * encoded in ebcdic, so we'll need to convert it to utf-8. */
+  const char ** argv_utf8;
+#endif
 
   /* Initialize the app. */
   if (svn_cmdline_init ("svn", stderr) != EXIT_SUCCESS)
@@ -831,7 +839,20 @@
     }
 
   /* Else, parse options. */
+#if !AS400_UTF8
   apr_getopt_init (&os, pool, argc, argv);
+#else
+  /* Convert argv to utf-8 for call to apr_getopt_init(). */
+  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
+  for (i = 0; i < argc; i++)
+    {
+      /* Use "0" for default job ccsid. */
+      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
+                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
+        return EXIT_FAILURE;
+    }
+  apr_getopt_init (&os, pool, argc, argv_utf8);
+#endif
   os->interleave = 1;
   while (1)
     {
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c  (revision 18362)
+++ subversion/svnadmin/main.c  (working copy)
@@ -37,6 +37,10 @@
 
 #include "svn_private_config.h"
 
+#if AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
+#endif
+
 
 /*** Code. ***/
 
@@ -1138,6 +1142,11 @@
   int opt_id;
   apr_array_header_t *received_opts;
   int i;
+#if AS400_UTF8
+  /* Even when compiled with UTF support in V5R4, argv is still
+   * encoded in ebcdic, so we'll need to convert it to utf-8. */
+  const char **argv_utf8;
+#endif
 
   /* Initialize the app. */
   if (svn_cmdline_init ("svnadmin", stderr) != EXIT_SUCCESS)
@@ -1179,7 +1188,20 @@
   opt_state.end_revision.kind = svn_opt_revision_unspecified;
 
   /* Parse options. */
+#if !AS400_UTF8
   apr_getopt_init (&os, pool, argc, argv);
+#else
+  /* Convert argv to utf-8 for call to apr_getopt_init(). */
+  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
+  for (i = 0; i < argc; i++)
+    {
+      /* Use "0" for default job ccsid. */
+      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
+                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
+        return EXIT_FAILURE;
+    }
+  apr_getopt_init (&os, pool, argc, argv_utf8);
+#endif
   os->interleave = 1;
 
   while (1)
Index: subversion/svndumpfilter/main.c
===================================================================
--- subversion/svndumpfilter/main.c     (revision 18362)
+++ subversion/svndumpfilter/main.c     (working copy)
@@ -34,6 +34,10 @@
 #include "svn_sorts.h"
 #include "svn_props.h"
 
+#if AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
+#endif
+
 
 /*** Code. ***/
 
@@ -1080,6 +1084,11 @@
   int opt_id;
   apr_array_header_t *received_opts;
   int i;
+#if AS400_UTF8
+  /* Even when compiled with UTF support in V5R4, argv is still
+   * encoded in ebcdic, so we'll need to convert it to utf-8. */
+  const char **argv_utf8;
+#endif
 
 
   /* Initialize the app. */
@@ -1122,7 +1131,20 @@
   opt_state.end_revision.kind = svn_opt_revision_unspecified;
 
   /* Parse options. */
+#if !AS400_UTF8
   apr_getopt_init (&os, pool, argc, argv);
+#else
+  /* Convert argv to utf-8 for call to apr_getopt_init(). */
+  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
+  for (i = 0; i < argc; i++)
+    {
+      /* Use "0" for default job ccsid. */
+      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
+                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
+        return EXIT_FAILURE;
+    }
+  apr_getopt_init (&os, pool, argc, argv_utf8);
+#endif
   os->interleave = 1;
   while (1)
     {
Index: subversion/svnlook/main.c
===================================================================
--- subversion/svnlook/main.c   (revision 18362)
+++ subversion/svnlook/main.c   (working copy)
@@ -44,6 +44,10 @@
 
 #include "svn_private_config.h"
 
+#if AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
+#endif
+
 
 /*** Some convenience macros and types. ***/
 
@@ -1987,6 +1991,11 @@
   int opt_id;
   apr_array_header_t *received_opts;
   int i;
+#if AS400_UTF8
+  /* Even when compiled with UTF support in V5R4, argv is still
+   * encoded in ebcdic, so we'll need to convert it to utf-8. */
+  const char **argv_utf8;
+#endif
 
   /* Initialize the app. */
   if (svn_cmdline_init ("svnlook", stderr) != EXIT_SUCCESS)
@@ -2027,7 +2036,20 @@
   opt_state.rev = SVN_INVALID_REVNUM;
 
   /* Parse options. */
+#if !AS400_UTF8
   apr_getopt_init (&os, pool, argc, argv);
+#else
+  /* Convert argv to utf-8 for call to apr_getopt_init(). */
+  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
+  for (i = 0; i < argc; i++)
+    {
+      /* Use "0" for default job ccsid. */
+      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
+                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
+        return EXIT_FAILURE;
+    }
+  apr_getopt_init (&os, pool, argc, argv_utf8);
+#endif
   os->interleave = 1;
   while (1)
     {
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c  (revision 18362)
+++ subversion/svnserve/main.c  (working copy)
@@ -100,6 +100,10 @@
 #define SVNSERVE_OPT_VERSION     260
 #define SVNSERVE_OPT_PID_FILE    261
 
+#if AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
+#endif
+
 static const apr_getopt_option_t svnserve__options[] =
   {
     {"daemon",           'd', 0, N_("daemon mode")},
@@ -273,6 +277,12 @@
   apr_status_t status;
   svn_ra_svn_conn_t *conn;
   apr_proc_t proc;
+#if AS400_UTF8
+  int i;
+  /* Even when compiled with UTF support in V5R4, argv is still
+   * encoded in ebcdic, so we'll need to convert it to utf-8. */
+  const char **argv_utf8;
+#endif
 #if APR_HAS_THREADS
   apr_threadattr_t *tattr;
   apr_thread_t *tid;
@@ -313,7 +323,20 @@
       return EXIT_FAILURE;
     }
 
+#if !AS400_UTF8
   apr_getopt_init(&os, pool, argc, argv);
+#else
+  /* Convert argv to utf-8 for call to apr_getopt_init(). */
+  argv_utf8 = apr_palloc(pool, sizeof(char *) * argc);
+  for (i = 0; i < argc; i++)
+    {
+      /* Use "0" for default job ccsid. */
+      if (svn_utf_cstring_to_utf8_ex(&argv_utf8[i], argv[i], "0",
+                                     SVN_UTF_ETOU_XLATE_HANDLE, pool))
+        return EXIT_FAILURE;
+    }
+  apr_getopt_init(&os, pool, argc, argv_utf8);
+#endif
 
   params.root = "/";
   params.tunnel = FALSE;
Index: subversion/svnsync/main.c
===================================================================
--- subversion/svnsync/main.c   (revision 18362)
+++ subversion/svnsync/main.c   (working copy)
@@ -23,6 +23,7 @@
 #include "svn_auth.h"
 #include "svn_opt.h"
 #include "svn_ra.h"
+#include "svn_utf.h"
 
 #include "svn_private_config.h"
 
@@ -38,6 +39,10 @@
 #define LAST_MERGED_REV_PROP   PROP_PREFIX "last-merged-rev"
 #define CURRENTLY_COPYING_PROP PROP_PREFIX "currently-copying"
 
+#if AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
+#endif
+
 static svn_opt_subcommand_t initialize_cmd,
                             synchronize_cmd,
                             copy_revprops_cmd,
@@ -1165,6 +1170,11 @@
   apr_pool_t *pool;
   svn_error_t *err;
   int opt_id, i;
+#if AS400_UTF8
+  /* Even when compiled with UTF support in V5R4, argv is still
+   * encoded in ebcdic, so we'll need to convert it to utf-8. */
+  const char **argv_utf8;
+#endif
 
   if (svn_cmdline_init ("svnsync", stderr) != EXIT_SUCCESS)
     {
@@ -1200,7 +1210,20 @@
       return EXIT_FAILURE;
     }
 
+#if !AS400_UTF8
   apr_getopt_init (&os, pool, argc, argv);
+#else
+  /* Convert argv to utf-8 for call to apr_getopt_init(). */
+  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
+  for (i = 0; i < argc; i++)
+    {
+      /* Use "0" for default job ccsid. */
+      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
+                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
+        return EXIT_FAILURE;
+    }
+  apr_getopt_init (&os, pool, argc, argv_utf8);
+#endif
 
   os->interleave = 1;
 
Index: subversion/svnversion/main.c
===================================================================
--- subversion/svnversion/main.c        (revision 18362)
+++ subversion/svnversion/main.c        (working copy)
@@ -25,6 +25,9 @@
 
 #define SVNVERSION_OPT_VERSION SVN_OPT_FIRST_LONGOPT_ID
 
+#if AS400_UTF8
+#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
+#endif
 
 static svn_error_t * version(apr_getopt_t *os, apr_pool_t *pool)
 {
@@ -124,6 +127,12 @@
       {"version", SVNVERSION_OPT_VERSION, 0, N_("show version 
information")},
       {0,             0,  0,  0}
     };
+#if AS400_UTF8
+  int i;
+  /* Even when compiled with UTF support in V5R4, argv is still
+   * encoded in ebcdic, so we'll need to convert it to utf-8. */
+  const char **argv_utf8;
+#endif
 
   /* Initialize the app. */
   if (svn_cmdline_init ("svnversion", stderr) != EXIT_SUCCESS)
@@ -163,7 +172,20 @@
     }
 #endif
 
+#if !AS400_UTF8
   apr_getopt_init(&os, pool, argc, argv);
+#else
+  /* Convert argv to utf-8 for call to apr_getopt_init(). */
+  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
+  for (i = 0; i < argc; i++)
+    {
+      /* Use "0" for default job ccsid. */
+      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
+                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
+        return EXIT_FAILURE;
+    }
+  apr_getopt_init (&os, pool, argc, argv_utf8);
+#endif
   os->interleave = 1;
   while (1)
     {

_____________________________________________________________________________
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


Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> 
>> So, argv is an array of pointers to const char, but the argv array 
>> itself is _not_ unmodifiable.
> 
> Interestingly enough, we have the same kind of wrong-headed code in 
> find_tunnel_agent() in libsvn_ra_svn/client.c ...

What, you think we don't need to allocate a new array there ... where we're 
adding three extra items to the end of it?

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Branko Čibej <br...@xbc.nu>.
Branko Čibej wrote:
> Paul Burba wrote:
>> 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!" :-)   
> The canonical prototype for main() is:
>
>    int main (int argc, const char *argv[]);
>
> So, argv is an array of pointers to const char, but the argv array 
> itself is _not_ unmodifiable.
Interestingly enough, we have the same kind of wrong-headed code in 
find_tunnel_agent() in libsvn_ra_svn/client.c ...

-- Brane


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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> 
> Sorry, I just missed this discussion Friday evening.  At least I can now 
> use Brane's suggestion.  Just one problem, the only main() function that 
> will currently allow us to change the argv pointers in place is 
> svnversion:
> 
>   svnversion:    int main (int argc, const char *argv[])
> 
> The other command line programs declare the pointers as const:
> 
>   svnlook:       int main (int argc, const char * const *argv)
>   svn:           int main (int argc, const char * const *argv)
>   svnadmin:      int main (int argc, const char * const *argv)
>   svndumpfilter: int main (int argc, const char * const *argv)
>   svnserve:      int main (int argc, const char * const *argv)
>   svnsync:       int main (int argc, const char * const argv[])
> 
> So, should I:
> 
> A) Use the previous approach and create a new array?
> 
> B) Modify the signatures of the offending main()s to
> 
>      int main(int argc, char *argv[])
>  
>    Brane, I'm not sure if your suggestion implied I should do this(?)

I'm happy with either way.  The more I think about modifying the signatures and 
using Brane's way, the more I like it: the conversion function's implementation 
and interface will both be simpler, and it will also be conveniently usable on 
a single string just by passing the address of a string pointer as "argv" and 1 
for "argc".

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Branko Čibej <br...@xbc.nu>.
Paul Burba wrote:
> Julian Foad <ju...@btopenworld.com> wrote on 02/10/2006 08:56:56 PM:
>
>   
>> Julian Foad wrote:
>>     
>>> Branko Čibej wrote:
>>>
>>>       
>>>> The correct way to modify the argv array is to replace the pointers 
>>>>         
> in 
>   
>>>> place; like this:
>>>>         
>>> That's _a_ correct way to do it, but providing a whole new array is 
>>>       
> also 
>   
>>> a correct way and has the advantage that [...]
>>>       
>> Nevertheless, I'm keen on minimalism and agree with you that replacing 
>>     
> the 
>   
>> pointers in place is better from that point of view; if our roles had 
>>     
> been 
>   
>> reversed I would probably have pointed it out.  :-)
>>
>> - Julian
>>     
>
> Hi All,
>
> Sorry, I just missed this discussion Friday evening.  At least I can now 
> use Brane's suggestion.  Just one problem, the only main() function that 
> will currently allow us to change the argv pointers in place is 
> svnversion:
>
>   svnversion:    int main (int argc, const char *argv[])
>
> The other command line programs declare the pointers as const:
>
>   svnlook:       int main (int argc, const char * const *argv)
>   svn:           int main (int argc, const char * const *argv)
>   svnadmin:      int main (int argc, const char * const *argv)
>   svndumpfilter: int main (int argc, const char * const *argv)
>   svnserve:      int main (int argc, const char * const *argv)
>   svnsync:       int main (int argc, const char * const argv[])
>
> So, should I:
>
> A) Use the previous approach and create a new array?
>
> B) Modify the signatures of the offending main()s to
>
>      int main(int argc, char *argv[])
>  
>    Brane, I'm not sure if your suggestion implied I should do this(?)
>   
I think doing this should be O.K. After all, main() isn't a public API, 
so we can change the signature as long as the change compiles with C rules.

I would take into account Mike's point about log messages being in a 
non-default encoding, though.

-- Brane


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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 13 Feb 2006, Paul Burba wrote:

>   svnversion:    int main (int argc, const char *argv[])
>
> The other command line programs declare the pointers as const:
>
>   svnlook:       int main (int argc, const char * const *argv)
>   svn:           int main (int argc, const char * const *argv)
>   svnadmin:      int main (int argc, const char * const *argv)
>   svndumpfilter: int main (int argc, const char * const *argv)
>   svnserve:      int main (int argc, const char * const *argv)
>   svnsync:       int main (int argc, const char * const argv[])
>
> So, should I:
>
> A) Use the previous approach and create a new array?
>
> B) Modify the signatures of the offending main()s to
>
>      int main(int argc, char *argv[])
>
>

I'm fine with B).

Thanks,
//Peter

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 02/10/2006 08:56:56 PM:

> Julian Foad wrote:
> > Branko Čibej wrote:
> > 
> >> The correct way to modify the argv array is to replace the pointers 
in 
> >> place; like this:
> > 
> > That's _a_ correct way to do it, but providing a whole new array is 
also 
> > a correct way and has the advantage that [...]
> 
> Nevertheless, I'm keen on minimalism and agree with you that replacing 
the 
> pointers in place is better from that point of view; if our roles had 
been 
> reversed I would probably have pointed it out.  :-)
> 
> - Julian

Hi All,

Sorry, I just missed this discussion Friday evening.  At least I can now 
use Brane's suggestion.  Just one problem, the only main() function that 
will currently allow us to change the argv pointers in place is 
svnversion:

  svnversion:    int main (int argc, const char *argv[])

The other command line programs declare the pointers as const:

  svnlook:       int main (int argc, const char * const *argv)
  svn:           int main (int argc, const char * const *argv)
  svnadmin:      int main (int argc, const char * const *argv)
  svndumpfilter: int main (int argc, const char * const *argv)
  svnserve:      int main (int argc, const char * const *argv)
  svnsync:       int main (int argc, const char * const argv[])

So, should I:

A) Use the previous approach and create a new array?

B) Modify the signatures of the offending main()s to

     int main(int argc, char *argv[])
 
   Brane, I'm not sure if your suggestion implied I should do this(?)

C) Other?

Paul B.

_____________________________________________________________________________
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


Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> Branko Čibej wrote:
> 
>> The correct way to modify the argv array is to replace the pointers in 
>> place; like this:
> 
> That's _a_ correct way to do it, but providing a whole new array is also 
> a correct way and has the advantage that [...]

Nevertheless, I'm keen on minimalism and agree with you that replacing the 
pointers in place is better from that point of view; if our roles had been 
reversed I would probably have pointed it out.  :-)

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> 
> The canonical prototype for main() is:
> 
>    int main (int argc, const char *argv[]);
> 
> So, argv is an array of pointers to const char, but the argv array 
> itself is _not_ unmodifiable.

Oh, OK.

> The correct way to modify the argv array is to replace the pointers in 
> place; like this:

That's _a_ correct way to do it, but providing a whole new array is also a 
correct way and has the advantage that a caller might want to keep the old 
array around to reference the unconverted strings, e.g. in the scenario of one 
of the arguments being passed in a different encoding from the rest, like 
possibly in an implementation that supports Mike Pilato's idea of "svn commit 
-m <Shift-JIS log message> --encoding Shift-JIS ...".

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Čibej <br...@xbc.nu> writes:

> The canonical prototype for main() is:
>
>     int main (int argc, const char *argv[]);

The C99 ISO standard explicitly allows (see 5.1.2.2.1/1)

     int main(void) { /* ... */ }

and

     int main(int argc, char *argv[]) { /* ... */ }

and states that any equivalent may be used.  There is a footnote that
states that "char ** argv" is equivalent to "char *argv[]".  I don't
have a copy of the earlier C90 standard, but I don't think it's any
different.

-- 
Philip Martin

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


Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Branko Čibej <br...@xbc.nu>.
Paul Burba wrote:
> 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!" :-) 
>   
The canonical prototype for main() is:

    int main (int argc, const char *argv[]);

So, argv is an array of pointers to const char, but the argv array 
itself is _not_ unmodifiable.

The correct way to modify the argv array is to replace the pointers in 
place; like this:

svn_error_t *
svn_cmdline_args_from_ebcdic (int argc, const char *argv[],
                              apr_pool_t *pool)
{
  int i;
  for (i = 0; i < argc; ++i)
    {
      char *arg_utf8;
      SVN_ERR(svn_utf_cstring_to_utf8_ex(&arg_utf, argv[i],
                                         SVN_UTF_ETOU_XLATE_HANDLE,
                                         pool);
      argv[i] = arg_utf8;
    }
  return SVN_NO_ERROR;
}

-- Brane


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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> 
> [[[
> 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

(Trivia: you might want to delete "on" or "under" at the end of that line.)

> 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

And you might as well do the seventh and final program, "svnversion", while 
you're at it - unless there is some reason why you deliberately left it out.

>    (main): Convert command line arguments from EBCDIC to UTF-8
>     with svn_cmdline_args_from_ebcdic().
> ]]]

[...]
> +svn_error_t *
> +svn_cmdline_args_from_ebcdic (int argc,
> +                              const char *const **argv_p,
> +                              apr_pool_t *pool)
> +{
> +  int i;
> +  const char **argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);

Sorry, one more thing that I didn't think of until now: argv should be an array 
of (argc + 1) entries, terminated by a null entry, because that's how it's 
defined (IIRC) in standard C, regardless of whether any of our programs 
currently rely on that.


> +  for (i = 0; i < argc; i++)
> +    {
> +      SVN_ERR (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], (*argv_p)[i], 
> "0",
> +                                           SVN_UTF_ETOU_XLATE_HANDLE,
> +                                           pool));
> +    }

So "argv_utf8[argc] = NULL;"

> +  *argv_p = argv_utf8;
> +  return SVN_NO_ERROR;
> +}

With at least the null-terminator fix, no need to post it here again, I'm happy 
for you to check this in.  (I believe you're supposed to put "Approved by: 
julianfoad" in the log message.)

Thanks.

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 02/10/2006 02:20:47 PM:

> Indeed casting away "const" is generally wrong.  You've just got a 
> bit confused 
> over the use of "const", which is hardly surprising with so many levels 
of 
> indirection going on in this pointer.  See below.
 
Following you direction I fixed it, new log and patch below.  Thanks!
 
> > 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. 
> 
> OK... maybe.  I'm afraid I don't understand the encoding parameters 
> so I can't 
> comment on that aspect.  This present patch does compile, though?

Yes, it does compile.  Hopefully my next patch will clear this up some.
 
> > Index: subversion/include/svn_cmdline.h
> > ===================================================================
> [...]
> > +#ifdef AS400_UTF8
> > +/** Convert each of the @argc strings in @argv from ebcdic to utf-8.
> > + * @a pool is used for all allocations.
> > + */
> 
> That would be, "@a argc" and "@a argv" for Doxygen notation.
>
> Let's rename the argument to "argv_p", as it's a pointer to what's 
normally 
> called "argv", and this is just too confusing otherwise!
>
> Be a bit more explicit about what this function actually does with its 
> parameters.  So, something like:
> 
>    /** Convert each of the @a argc strings in @a *argv_p from ebcdicto 
utf-8.
>     * Replace @a *argv_p with a pointer to a new array of the new 
strings, all
>     * allocated in @a pool.
>     */
> 
> (or being even more explicit wouldn't hurt; this is still a bit opaque).

Spelled it out a bit clearer, with valid Doxygen notation.
 
> > +svn_error_t *
> > +svn_cmdline_args_from_ebcdic (int argc,
> > +                              char * const * const *argv,
> > +                              apr_pool_t *pool);
> 
> I think you've got the "const"s in the wrong place; this presently says 
that 
> the individual characters are writable (which isn't what's wanted) 
> and that the 
> argument itself points to something constant, whereas you want to 
> write to what 
> the argument points to.  So:
> 
>    const char *const **argv_p,
> 
> 
> > 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"
> 
> In this value (between the quotes), can we have all dashes rather than 
mixing 
> with some underscores, to match the style of the existing handles just 
above.

Sorry that was a typo, fixed.

> > @@ -446,3 +450,22 @@
> > +
> > +#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);
> 
> Then if you declare this as "const char **"...
> 
> > +  for (i = 0; i < argc; i++)
> > +    {
> > +      SVN_ERR (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], (*argv)[i], 
"0",
> 
> ... it will firstly pass the right kind of pointer as the first arg 
> here (I was 
> getting a warning) ...
> 
> > +                                           SVN_UTF_ETOU_XLATE_HANDLE,
> > +                                           pool));
> > +    }
> > +  *((char***)argv) = argv_utf8;
> 
> ... and then you won't need any cast at all here, and all is well.

That works, thanks for the pointers <cue collective groan from the crowd>, 
I knew I was doing *something* screwy.
 
> > +  return SVN_NO_ERROR;
> > +}
> > +#endif
> > \ No newline at end of file
> 
> Oops - please let the last line of the file end with a newline 
character.  It 
> would be a trivial matter except that it's an error to some 
> compilers (at least 
> it was an error if it occurred on a preprocessor line on one compiler 
that I 
> used to use).

Done.

[[[
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,17 @@
                               void *cancel_baton,
                               apr_pool_t *pool);
 
+#ifdef AS400_UTF8
+/** Convert each of the @a argc strings in @a *argv_p from EBCDIC to 
UTF-8,
+  * storing each UTF-8 string in a new array of strings allocated in
+  * @a pool.  Replace @a *argv_p with the new array of UTF-8 strings.
+  */
+svn_error_t *
+svn_cmdline_args_from_ebcdic (int argc,
+                              const char *const **argv_p,
+                              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,23 @@
 
   return SVN_NO_ERROR;
 }
+
+#ifdef AS400_UTF8
+svn_error_t *
+svn_cmdline_args_from_ebcdic (int argc,
+                              const char *const **argv_p,
+                              apr_pool_t *pool)
+{
+  int i;
+  const 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_p)[i], 
"0",
+                                           SVN_UTF_ETOU_XLATE_HANDLE,
+                                           pool));
+    }
+  *argv_p = argv_utf8;
+  return SVN_NO_ERROR;
+}
+#endif
+
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)
@@ -1200,6 +1200,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);
 
   os->interleave = 1;


_____________________________________________________________________________
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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> 
> Ouch, I always thought redundant code was a good thing ;-)

Heh!  (Actually, that reminds me, one day I want to work on software that does 
make use of redundancy for improved reliability.  Obviously that requires a 
monitoring function that checks if one of the multiple memories/algorithms/etc. 
gives a different result from the others.  The idea is used for fault-tolerant 
RAID storage, but that's only redundancy in data, not code.  With redundant 
self-healing code you can write software that can run from RAM that is 
suffering random corruptions from cosmic rays or faulty hardware or whatever. 
Of course the monitoring function itself must be redundantly distributed. 
Interesting stuff.)


> [...] 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!" :-) 

Indeed casting away "const" is generally wrong.  You've just got a bit confused 
over the use of "const", which is hardly surprising with so many levels of 
indirection going on in this pointer.  See below.


> 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. 

OK... maybe.  I'm afraid I don't understand the encoding parameters so I can't 
comment on that aspect.  This present patch does compile, though?

> 
> [[[
> 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.

Nice.

> 
> * 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
> ===================================================================
[...]
> +#ifdef AS400_UTF8
> +/** Convert each of the @argc strings in @argv from ebcdic to utf-8.
> + * @a pool is used for all allocations.
> + */

That would be, "@a argc" and "@a argv" for Doxygen notation.

Let's rename the argument to "argv_p", as it's a pointer to what's normally 
called "argv", and this is just too confusing otherwise!

Be a bit more explicit about what this function actually does with its 
parameters.  So, something like:

   /** Convert each of the @a argc strings in @a *argv_p from ebcdic to utf-8.
    * Replace @a *argv_p with a pointer to a new array of the new strings, all
    * allocated in @a pool.
    */

(or being even more explicit wouldn't hurt; this is still a bit opaque).

> +svn_error_t *
> +svn_cmdline_args_from_ebcdic (int argc,
> +                              char * const * const *argv,
> +                              apr_pool_t *pool);

I think you've got the "const"s in the wrong place; this presently says that 
the individual characters are writable (which isn't what's wanted) and that the 
argument itself points to something constant, whereas you want to write to what 
the argument points to.  So:

   const char *const **argv_p,


> 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"

In this value (between the quotes), can we have all dashes rather than mixing 
with some underscores, to match the style of the existing handles just above.


> @@ -446,3 +450,22 @@
> +
> +#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);

Then if you declare this as "const char **"...

> +  for (i = 0; i < argc; i++)
> +    {
> +      SVN_ERR (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], (*argv)[i], "0",

... it will firstly pass the right kind of pointer as the first arg here (I was 
getting a warning) ...

> +                                           SVN_UTF_ETOU_XLATE_HANDLE,
> +                                           pool));
> +    }
> +  *((char***)argv) = argv_utf8;

... and then you won't need any cast at all here, and all is well.

> +  return SVN_NO_ERROR;
> +}
> +#endif
> \ No newline at end of file

Oops - please let the last line of the file end with a newline character.  It 
would be a trivial matter except that it's an error to some compilers (at least 
it was an error if it occurred on a preprocessor line on one compiler that I 
used to use).

> 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
> +

Beautiful.

>    /* Else, parse options. */
>    apr_getopt_init (&os, pool, argc, argv);
>    os->interleave = 1;

Thanks.  Nearly there.

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> 
> This is the first of several (probably 5 - 10) patch submissions that will 
> allow subversion to run on the IBM iSeries under OS400 V5R4.  I'll try to 
> keep each patch relatively small and focused on one problem.

Excellent, we appreciate the effort to make the patches focused.

Sorry, I've written rather a lot of criticism below about what is really a 
rather straightforward change.


> [[[

OK, here we want to insert a description of the change and its purpose, 
something like:

   OS400/EBCDIC port: convert command-line arguments from EBCDIC to UTF-8.
   This is one of several patches to allow Subversion to run on the IBM iSeries
   under OS400 V5R4.

> * subversion/svn/main.c:
> * subversion/svnadmin/main.c 
> * subversion/svndumpfilter/main.c
> * subversion/svnlook/main.c
> * subversion/svnserve/main.c
> * subversion/svnsync/main.c
> * subversion/svnversion/main.c
>    (SVN_UTF_ETOU_XLATE_HANDLE): Xlate key for ebcdic (CCSID 0) to utf-8
>     conversions.
>    (main): Convert incoming argv strings to utf-8.  Note that 
>     svn_utf_cstring_to_utf8_ex() is used instead of 
> svn_utf_cstring_to_utf8
>     because V5R4 thinks native strings are in utf-8.

That note addresses an implementation detail so would be better in the code 
than in the log message.

> ]]]
> 
> Index: subversion/svn/main.c
> ===================================================================
> --- subversion/svn/main.c       (revision 18362)
> +++ subversion/svn/main.c       (working copy)
> @@ -48,6 +48,9 @@
>  
>  #include "svn_private_config.h"
>  
> +#if AS400_UTF8

Do we already have the "configure" magic that sets this macro to true or false 
on every system?  If not (if it is set for AS400_UTF8 systems and unset 
elsewhere) then the directive should be "ifdef".  Or is it true on some 
systems, false on others, and unset on others, in which case a two-level test 
might be needed?  Basically I don't want us using "#if" to test something that 
may not be set, even though it is legal in C.

> +#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
> +#endif

>  /*** Option Processing ***/
>  
> @@ -780,6 +783,11 @@
>    svn_cl__cmd_baton_t command_baton;
>    svn_auth_baton_t *ab;
>    svn_config_t *cfg;
> +#if AS400_UTF8
> +  /* Even when compiled with UTF support in V5R4, argv is still
> +   * encoded in ebcdic, so we'll need to convert it to utf-8. */
> +  const char ** argv_utf8;
> +#endif
>  
>    /* Initialize the app. */
>    if (svn_cmdline_init ("svn", stderr) != EXIT_SUCCESS)
> @@ -831,7 +839,20 @@
>      }
>  
>    /* Else, parse options. */
> +#if !AS400_UTF8
>    apr_getopt_init (&os, pool, argc, argv);
> +#else
> +  /* Convert argv to utf-8 for call to apr_getopt_init(). */
> +  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
> +  for (i = 0; i < argc; i++)
> +    {
> +      /* Use "0" for default job ccsid. */

I'm not sure if that comment adds anything: if "default job ccsid" is just a 
description of the argument to the function then the code already says that.  A 
comment saying why would be of more use (e.g. "The [default] job ccsid isn't 
used, it just has to look like a number").

> +      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
> +                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
> +        return EXIT_FAILURE;
> +    }
> +  apr_getopt_init (&os, pool, argc, argv_utf8);
> +#endif

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 as possible. 
  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.

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 this change 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, it we 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.

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".

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Paul Burba <pa...@softlanding.com>.
rooneg@gmail.com wrote on 02/13/2006 01:16:25 PM:

> On 2/13/06, Paul Burba <pa...@softlanding.com> wrote:
> 
> >    /* Else, parse options. */
> > -  apr_getopt_init (&os, pool, argc, argv);
> > +  svn_cmdline_getopt_init (&os, pool, argc, argv);
> 
> If svn_cmdline_getopt_init is going to return a svn_error_t it really
> needs to be checked, rather than just ignored.  Not that the existing
> code was doing so well in that department mind you, but it's still
> wrong.
> 
> -garrett

Hi Garrett,

Assuming the rest of the patch gets a clean bill of health, I'll add the 
following for each main() function, with the appropriate "svn*: " prefix 
for each of course.

-  apr_getopt_init (&os, pool, argc, argv);
+  err = svn_cmdline_getopt_init (&os, pool, argc, argv);
+  if (err)
+    return svn_cmdline_handle_exit_error (err, pool, "svn: ");

Thanks,

Paul B.

_____________________________________________________________________________
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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 2/13/06, Paul Burba <pa...@softlanding.com> wrote:

>    /* Else, parse options. */
> -  apr_getopt_init (&os, pool, argc, argv);
> +  svn_cmdline_getopt_init (&os, pool, argc, argv);

If svn_cmdline_getopt_init is going to return a svn_error_t it really
needs to be checked, rather than just ignored.  Not that the existing
code was doing so well in that department mind you, but it's still
wrong.

-garrett

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


Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> 
> Just wanted to send out a humble reminder regarding this patch.  It was 
> the first of the OS400/EBCDIC patches and has gone through a few 
> iterations while subsequent OS400 patches were committed.  My most recent 
> changes have been kicking around for almost a week and I haven't heard 
> anything.  I hope it addresses everyone's concerns(?).  If there are any 
> further tweaks necessary on my part please let me know, if it looks good 
> I'd like to get it committed.

Thanks for the reminder.  It has my approval.  If nobody objects today, go 
ahead and commit it.

Tiny nit: the log message says this is "the second of several patches"; it is 
now at least the third to be applied.

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Paul Burba <pa...@softlanding.com>.
Hello,

Just wanted to send out a humble reminder regarding this patch.  It was 
the first of the OS400/EBCDIC patches and has gone through a few 
iterations while subsequent OS400 patches were committed.  My most recent 
changes have been kicking around for almost a week and I haven't heard 
anything.  I hope it addresses everyone's concerns(?).  If there are any 
further tweaks necessary on my part please let me know, if it looks good 
I'd like to get it committed.

Thanks all,

Paul B.

P.S. The attached patch differs from last weeks only in that it uses the 
no-space-before-paren format.

Julian Foad <ju...@btopenworld.com> wrote on 02/13/2006 02:38:01 PM:

> Paul Burba wrote:

> > -main (int argc, const char * const *argv)
> > +main(int argc, char *argv[])
> [...]
> > -  apr_getopt_init (&os, pool, argc, argv);
> > +  svn_cmdline_getopt_init (&os, pool, argc, argv);
> 
> subversion/svn/main.c:834: warning: passing argument 4 of 
> 'svn_cmdline_getopt_init' from incompatible pointer type
> 
> I think it's the C language and/or compiler that's being a bit 
unreasonable 
> about pointer compatibility here, but nevertheless I try to avoid 
> this situation.
> 
> Could we compromise and make it:
> 
>    main(int argc, const char *argv[])
> 
> That won't be quite as the C standard says, but it will be a step closer 

than 
> what we have now.  An alternative is to remove "const" from the 
signature of 
> svn_cmdline_getopt_init(), but that would require a cast inside the 
function 
> when calling apr_getopt_init.

New patch attached uses your first suggestion, is that acceptable to 
everyone?
 
> Secondly, I'm a little uneasy about introducing a public API that's 
> documented 
> in terms of what it does on one particular platform, and that we maywell 

want 
> to change soon to do stuff on other platforms and/or to handle the "-m" 
> argument differently.  Can we make it private just by including a double 


> underscore in it and adding a note to that effect?
> 
>    /* [...]  This is a private API for Subversion's own use. */
>    svn_cmdline__getopt_init ...
> 
> (We've previously been unable to decide in exactly what ways we can 
> use private 
> symbols, but I hope this usage won't be objectionable.)

This is also in the latest patch, again is this acceptable to you all?
 
> Later you wrote:
> > -  apr_getopt_init (&os, pool, argc, argv);
> > +  err = svn_cmdline_getopt_init (&os, pool, argc, argv);
> > +  if (err)
> > +    return svn_cmdline_handle_exit_error (err, pool, "svn: ");
> 
> +1 on that.

Added similar error handling to all callers of svn_cmdline__getopt_init. 
 
[[[
OS400/EBCDIC Port: Convert command-line arguments from EBCDIC to UTF-8.

This is the second of several patches to allow Subversion to run on 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.

To avoid const restrictions and allow EBCDIC to UTF-8 conversion of 
command line arguments in place, the signature of main() in all command
line programs is standardized to main(int argc, const char *argv[]).

Approved by: Julian Foad <ju...@btopenworld.com>

Suggestions by: Julian, Philip Martin <ph...@codematters.co.uk>, and
                Brane <br...@xbc.nu>
 
* subversion/include/svn_cmdline.h
   Include apr_getopt.h
   (svn_cmdline__getopt_init): 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__getopt_init): 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
* subversion/svnversion/main.c
   (main): Standardize signature and replace call to apr_getopt_init()
    with new wrapper function svn_cmdline__getopt_init().
]]]



_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 02/13/2006 02:38:01 PM:

> Paul Burba wrote:

> > -main (int argc, const char * const *argv)
> > +main(int argc, char *argv[])
> [...]
> > -  apr_getopt_init (&os, pool, argc, argv);
> > +  svn_cmdline_getopt_init (&os, pool, argc, argv);
> 
> subversion/svn/main.c:834: warning: passing argument 4 of 
> 'svn_cmdline_getopt_init' from incompatible pointer type
> 
> I think it's the C language and/or compiler that's being a bit 
unreasonable 
> about pointer compatibility here, but nevertheless I try to avoid 
> this situation.
> 
> Could we compromise and make it:
> 
>    main(int argc, const char *argv[])
> 
> That won't be quite as the C standard says, but it will be a step closer 
than 
> what we have now.  An alternative is to remove "const" from the 
signature of 
> svn_cmdline_getopt_init(), but that would require a cast inside the 
function 
> when calling apr_getopt_init.

New patch attached uses your first suggestion, is that acceptable to 
everyone?
 
> Secondly, I'm a little uneasy about introducing a public API that's 
> documented 
> in terms of what it does on one particular platform, and that we maywell 
want 
> to change soon to do stuff on other platforms and/or to handle the "-m" 
> argument differently.  Can we make it private just by including a double 

> underscore in it and adding a note to that effect?
> 
>    /* [...]  This is a private API for Subversion's own use. */
>    svn_cmdline__getopt_init ...
> 
> (We've previously been unable to decide in exactly what ways we can 
> use private 
> symbols, but I hope this usage won't be objectionable.)

This is also in the latest patch, again is this acceptable to you all?
 
> Later you wrote:
> > -  apr_getopt_init (&os, pool, argc, argv);
> > +  err = svn_cmdline_getopt_init (&os, pool, argc, argv);
> > +  if (err)
> > +    return svn_cmdline_handle_exit_error (err, pool, "svn: ");
> 
> +1 on that.

Added similar error handling to all callers of svn_cmdline__getopt_init. 
 
[[[
OS400/EBCDIC Port: Convert command-line arguments from EBCDIC to UTF-8.

This is the second of several patches to allow Subversion to run on 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.

To avoid const restrictions and allow EBCDIC to UTF-8 conversion of 
command line arguments in place, the signature of main() in all command
line programs is standardized to main(int argc, const char *argv[]).

Approved by: Julian Foad <ju...@btopenworld.com>

Suggestions by: Julian, Philip Martin <ph...@codematters.co.uk>, and
                Brane <br...@xbc.nu>
 
* subversion/include/svn_cmdline.h
   Include apr_getopt.h
   (svn_cmdline__getopt_init): 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__getopt_init): 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
* subversion/svnversion/main.c
   (main): Standardize signature and replace call to apr_getopt_init()
    with new wrapper function svn_cmdline__getopt_init().
]]]



_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> 
> You said to commit this last Friday, but given all the changes since then 
> I thought it best to post the log and patch again, to make sure this is 
> what everyone had in mind.

Thanks.


> -main (int argc, const char * const *argv)
> +main(int argc, char *argv[])
[...]
> -  apr_getopt_init (&os, pool, argc, argv);
> +  svn_cmdline_getopt_init (&os, pool, argc, argv);

subversion/svn/main.c:834: warning: passing argument 4 of 
'svn_cmdline_getopt_init' from incompatible pointer type

I think it's the C language and/or compiler that's being a bit unreasonable 
about pointer compatibility here, but nevertheless I try to avoid this situation.

Could we compromise and make it:

   main(int argc, const char *argv[])

That won't be quite as the C standard says, but it will be a step closer than 
what we have now.  An alternative is to remove "const" from the signature of 
svn_cmdline_getopt_init(), but that would require a cast inside the function 
when calling apr_getopt_init.


Secondly, I'm a little uneasy about introducing a public API that's documented 
in terms of what it does on one particular platform, and that we may well want 
to change soon to do stuff on other platforms and/or to handle the "-m" 
argument differently.  Can we make it private just by including a double 
underscore in it and adding a note to that effect?

   /* [...]  This is a private API for Subversion's own use. */
   svn_cmdline__getopt_init ...

(We've previously been unable to decide in exactly what ways we can use private 
symbols, but I hope this usage won't be objectionable.)


Later you wrote:
> -  apr_getopt_init (&os, pool, argc, argv);
> +  err = svn_cmdline_getopt_init (&os, pool, argc, argv);
> +  if (err)
> +    return svn_cmdline_handle_exit_error (err, pool, "svn: ");

+1 on that.


- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 02/13/2006 09:24:07 AM:

> Philip Martin wrote:
> > 
> > Lets try and keep that conditional code in one place, rather than in
> > every main faunction.  How about a function svn_getopt_int that does
> > all that on EBCDIC platforms and just calls apr_getopt_init on the
> > others?  Put it in libsvn_subr/cmdline.c perhaps?
> 
> Sorry, I missed the significance of this before.  Yes, good idea. 
> That will be 
> very neat.
> 
> - Julian

Julian (& company),

You said to commit this last Friday, but given all the changes since then 
I thought it best to post the log and patch again, to make sure this is 
what everyone had in mind.

Let me know,

Thanks, 

Paul B.

[[[
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 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.

To avoid const restrictions and allow EBCDIC to UTF-8 conversion of 
command
line arguments in place, this patch also normalizes the signature of 
main()
to the C99 standard, i.e. int main (int argc, char *argv[]), for all 
command
line programs.

Approved by: Julian Foad <ju...@btopenworld.com>

Suggestions by: Julian, Philip Martin <ph...@codematters.co.uk>, and
                Brane <br...@xbc.nu>
 
* subversion/include/svn_cmdline.h
   Include apr_getopt.h
   (svn_cmdline_getopt_init): 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_getopt_init): 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
* subversion/svnversion/main.c
   (main): Change signature to C99 standard.  Replace call to
    apr_getopt_init() with new wrapper function svn_cmdline_getopt_init().
]]]

Index: subversion/include/svn_cmdline.h
===================================================================
--- subversion/include/svn_cmdline.h    (revision 18446)
+++ subversion/include/svn_cmdline.h    (working copy)
@@ -29,6 +29,7 @@
 #define APR_WANT_STDIO
 #endif
 #include <apr_want.h>
+#include <apr_getopt.h>
 
 #include "svn_utf.h"
 #include "svn_auth.h"
@@ -247,6 +248,18 @@
                               void *cancel_baton,
                               apr_pool_t *pool);
 
+/** Wrapper for apr_getopt_init(), which see.
+ * 
+ * On OS400 V5R4, prior to calling apr_getopt_init(), converts each of 
the
+ * @a argc strings in @a argv[] in place from EBCDIC to UTF-8, allocating
+ * each new UTF-8 string in @a pool.
+ */
+svn_error_t *
+svn_cmdline_getopt_init (apr_getopt_t **os,
+                         apr_pool_t *pool,
+                         int argc,
+                         const char *argv[]);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c    (revision 18446)
+++ 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,30 @@
 
   return SVN_NO_ERROR;
 }
+
+svn_error_t *
+svn_cmdline_getopt_init(apr_getopt_t **os,
+                        apr_pool_t *pool,
+                        int argc,
+                        const char *argv[])
+{
+  apr_status_t apr_err;
+#ifdef AS400_UTF8
+  /* Convert command line args from EBCDIC to UTF-8. */
+  int i;
+  for (i = 0; i < argc; ++i)
+    {
+      char *arg_utf8;
+      SVN_ERR(svn_utf_cstring_to_utf8_ex(&arg_utf8, argv[i], "0",
+                                         SVN_UTF_ETOU_XLATE_HANDLE,
+                                         pool));
+      argv[i] = arg_utf8;
+    }
+#endif
+  apr_err = apr_getopt_init (os, pool, argc, argv);
+  if (apr_err)
+    return svn_error_wrap_apr (apr_err,
+                               _("Error initializing command line 
arguments"));
+  return SVN_NO_ERROR;
+}
+
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c       (revision 18446)
+++ subversion/svn/main.c       (working copy)
@@ -762,7 +762,7 @@
 /*** Main. ***/
 
 int
-main (int argc, const char * const *argv)
+main(int argc, char *argv[])
 {
   svn_error_t *err;
   apr_allocator_t *allocator;
@@ -831,7 +831,7 @@
     }
 
   /* Else, parse options. */
-  apr_getopt_init (&os, pool, argc, argv);
+  svn_cmdline_getopt_init (&os, pool, argc, argv);
   os->interleave = 1;
   while (1)
     {
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c  (revision 18446)
+++ subversion/svnadmin/main.c  (working copy)
@@ -1136,7 +1136,7 @@
 /** Main. **/
 
 int
-main (int argc, const char * const *argv)
+main(int argc, char *argv[])
 {
   svn_error_t *err;
   apr_status_t apr_err;
@@ -1190,7 +1190,7 @@
   opt_state.end_revision.kind = svn_opt_revision_unspecified;
 
   /* Parse options. */
-  apr_getopt_init (&os, pool, argc, argv);
+  svn_cmdline_getopt_init (&os, pool, argc, argv);
   os->interleave = 1;
 
   while (1)
Index: subversion/svndumpfilter/main.c
===================================================================
--- subversion/svndumpfilter/main.c     (revision 18446)
+++ subversion/svndumpfilter/main.c     (working copy)
@@ -1067,7 +1067,7 @@
 /** Main. **/
 
 int
-main (int argc, const char * const *argv)
+main(int argc, char *argv[])
 {
   svn_error_t *err;
   apr_status_t apr_err;
@@ -1122,7 +1122,7 @@
   opt_state.end_revision.kind = svn_opt_revision_unspecified;
 
   /* Parse options. */
-  apr_getopt_init (&os, pool, argc, argv);
+  svn_cmdline_getopt_init (&os, pool, argc, argv);
   os->interleave = 1;
   while (1)
     {
Index: subversion/svnlook/main.c
===================================================================
--- subversion/svnlook/main.c   (revision 18446)
+++ subversion/svnlook/main.c   (working copy)
@@ -1974,7 +1974,7 @@
 /*** Main. ***/
 
 int
-main (int argc, const char * const *argv)
+main(int argc, char *argv[])
 {
   svn_error_t *err;
   apr_status_t apr_err;
@@ -2027,7 +2027,7 @@
   opt_state.rev = SVN_INVALID_REVNUM;
 
   /* Parse options. */
-  apr_getopt_init (&os, pool, argc, argv);
+  svn_cmdline_getopt_init (&os, pool, argc, argv);
   os->interleave = 1;
   while (1)
     {
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c  (revision 18446)
+++ subversion/svnserve/main.c  (working copy)
@@ -256,7 +256,7 @@
 }
 
 
-int main(int argc, const char *const *argv)
+int main(int argc, char *argv[])
 {
   enum run_mode run_mode = run_mode_unspecified;
   svn_boolean_t foreground = FALSE;
@@ -313,7 +313,7 @@
       return EXIT_FAILURE;
     }
 
-  apr_getopt_init(&os, pool, argc, argv);
+  svn_cmdline_getopt_init(&os, pool, argc, argv);
 
   params.root = "/";
   params.tunnel = FALSE;
Index: subversion/svnsync/main.c
===================================================================
--- subversion/svnsync/main.c   (revision 18446)
+++ subversion/svnsync/main.c   (working copy)
@@ -1154,7 +1154,7 @@
 }
 
 int
-main (int argc, const char * const argv[])
+main(int argc, char *argv[])
 {
   const svn_opt_subcommand_desc_t *subcommand = NULL;
   apr_array_header_t *received_opts;
@@ -1200,7 +1200,7 @@
       return EXIT_FAILURE;
     }
 
-  apr_getopt_init (&os, pool, argc, argv);
+  svn_cmdline_getopt_init (&os, pool, argc, argv);
 
   os->interleave = 1;
 
Index: subversion/svnversion/main.c
===================================================================
--- subversion/svnversion/main.c        (revision 18446)
+++ subversion/svnversion/main.c        (working copy)
@@ -106,7 +106,7 @@
  * program.  Obviously we don't want to have to run svn when building 
svn.
  */
 int
-main(int argc, const char *argv[])
+main(int argc, char *argv[])
 {
   const char *wc_path, *trail_url;
   apr_allocator_t *allocator;
@@ -163,7 +163,7 @@
     }
 #endif
 
-  apr_getopt_init(&os, pool, argc, argv);
+  svn_cmdline_getopt_init(&os, pool, argc, argv);
   os->interleave = 1;
   while (1)
     {


_____________________________________________________________________________
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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> 
> Lets try and keep that conditional code in one place, rather than in
> every main faunction.  How about a function svn_getopt_int that does
> all that on EBCDIC platforms and just calls apr_getopt_init on the
> others?  Put it in libsvn_subr/cmdline.c perhaps?

Sorry, I missed the significance of this before.  Yes, good idea.  That will be 
very neat.

- Julian

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

Re: [PATCH] #1 Update on port to OS400/EBCDIC

Posted by Philip Martin <ph...@codematters.co.uk>.
Paul Burba <pa...@softlanding.com> writes:

> --- subversion/svn/main.c       (revision 18362)
> +++ subversion/svn/main.c       (working copy)
> @@ -48,6 +48,9 @@
>  
>  #include "svn_private_config.h"
>  
> +#if AS400_UTF8
> +#define SVN_UTF_ETOU_XLATE_HANDLE "svn-utf-etou-xlate-handle"
> +#endif
>  
>  /*** Option Processing ***/
>  
> @@ -780,6 +783,11 @@
>    svn_cl__cmd_baton_t command_baton;
>    svn_auth_baton_t *ab;
>    svn_config_t *cfg;
> +#if AS400_UTF8
> +  /* Even when compiled with UTF support in V5R4, argv is still
> +   * encoded in ebcdic, so we'll need to convert it to utf-8. */
> +  const char ** argv_utf8;
> +#endif
>  
>    /* Initialize the app. */
>    if (svn_cmdline_init ("svn", stderr) != EXIT_SUCCESS)
> @@ -831,7 +839,20 @@
>      }
>  
>    /* Else, parse options. */
> +#if !AS400_UTF8
>    apr_getopt_init (&os, pool, argc, argv);
> +#else
> +  /* Convert argv to utf-8 for call to apr_getopt_init(). */
> +  argv_utf8 = apr_palloc (pool, sizeof(char *) * argc);
> +  for (i = 0; i < argc; i++)
> +    {
> +      /* Use "0" for default job ccsid. */
> +      if (svn_utf_cstring_to_utf8_ex (&argv_utf8[i], argv[i], "0",
> +                                      SVN_UTF_ETOU_XLATE_HANDLE, pool))
> +        return EXIT_FAILURE;
> +    }
> +  apr_getopt_init (&os, pool, argc, argv_utf8);
> +#endif
>    os->interleave = 1;
>    while (1)
>      {

Lets try and keep that conditional code in one place, rather than in
every main faunction.  How about a function svn_getopt_int that does
all that on EBCDIC platforms and just calls apr_getopt_init on the
others?  Put it in libsvn_subr/cmdline.c perhaps?

-- 
Philip Martin

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