Charles, Regis, Some comments regarding this commit...
In message <20090806010230.9dd532388...@eris.apache.org>, regi...@apache.org writes: > > Author: regisxu > Date: Thu Aug 6 01:02:29 2009 > New Revision: 801484 > > URL: http://svn.apache.org/viewvc?rev=801484&view=rev > Log: > Apply patch for HARMONY-6279: [classlib][luni] file.encoding is always set to > ISO-9959-1 if using drlvm > [snip] > Modified: modules/luni/src/main/native/luni/unix/helpers.c > ============================================================================= > --- modules/luni/src/main/native/luni/unix/helpers.c (original) > +++ modules/luni/src/main/native/luni/unix/helpers.c Thu Aug 6 01:02:29 2009 > @@ -220,3 +220,41 @@ > > hysock_setopt_bool (socketP, HY_SOL_SOCKET, HY_SO_REUSEADDR, &value); > } > + > +/* Get charset from the OS */ > +inline void getOSCharset(char *locale, const size_t size) { > + char * codec = NULL; > + size_t cur = 0; > + short flag = 0; > + setlocale(LC_CTYPE, ""); > + codec = setlocale(LC_CTYPE, NULL); > + // get codeset from language[_territory][.codese...@modifier] > + while (*codec) { > + if (!flag) { > + if (*codec != '.') { > + codec++; > + continue; > + } else { > + flag = 1; > + codec++; > + } > + } else { > + if (*codec == '@') { > + break; > + } else { > + locale[cur++] = (*codec); > + codec++; > + if (cur >= size) { > + // Not enough size > + cur = 0; > + break; > + } > + } > + } > + } > + locale[cur] = '\0'; > + if (!strlen(locale)) { > + strcpy(locale, "8859_1"); > + } > + return; > +} The inline keyword causes a compiler error on AIX: "helpers.c", line 225.1: 1506-485 (S) Parameter declaration list is incompatible with declarator for inline. I was going to #ifdef out the inline for AIX. However, I'm not convinced inline makes sense here even for Linux. In -Dhy.cfg=debug mode there are no optimizations so this function is never inlined, but even in -Dhy.cfg=release mode none of the gcc versions I tried inlined this function. So, I see no benefit from having it. I suggest we remove it for portability. Or is there a benefit that I am missing? > Added: modules/luni/src/main/native/luni/windows/charsetmap.h > --- modules/luni/src/main/native/luni/windows/charsetmap.h (added) > +++ modules/luni/src/main/native/luni/windowscharsetmap.h Thu Aug 6 01:02:29 > @@ -0,0 +1,123 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one or more > + * contributor license agreements. See the NOTICE file distributed with > + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache License, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > +/** > + * @author Charles Lee > + * @version $Revision: 1.0$ > + */ What is this Revision? I don't think it makes sense in this context. I notice that there are '$Revision...' tags in over 3k files in classlib alone but since we don't have svn keyword expansion enabled (and I hope we never do) then most are either not expanded or they were expanded elsewhere and are just misleading in the context of our svn repository. I suggest we remove them? (I'd rather not see author tags either but I'm not sure everyone agrees with this. There is some discussion in the list archives.) > [snip] > Modified: modules/luni/src/main/native/luni/windows/helpers.c > --- modules/luni/src/main/native/luni/windows/helpers.c (original) > +++ modules/luni/src/main/native/luni/windows/helpers.c Thu Aug 6 01:02:29 > 2009 > @@ -25,12 +25,14 @@ > #include <windows.h> > #include <winbase.h> > #include <stdlib.h> > +#include <stdio.h> > #include <LMCONS.H> > #include <direct.h> I don't have a windows machine configured to build Harmony right now but I suspect that stdio.h was only required for your debugging and can/should be removed from any patches/commits? Regards, Mark.
