Thanks Charles. I've made those changes. What about the more important 'inline' issue?
Regards, Mark. In message <5948b71e0908060209g2c844f83sc2a920c1ddfe3...@mail.gmail.com>, > Charles Lee writes: > > On Thu, Aug 6, 2009 at 4:54 PM, Mark Hindess <mark.hind...@googlemail.com> > wrote: > > > 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.) > > > I'm OK. Please delete it. > > > > > > > > [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? > > > I have compiled it on the windows. Seems I forget to delete it when deleted > my debug function. Please remove it. Thanks. > > > > > Regards, > > Mark. > -- > Yours sincerely, > Charles Lee
