Hi Mark, I add inline just want it to be what it supposed to be. Since it is just a hint, remove it will be ok.
By the way, why can't AIX recognize the 'inline'? On Thu, Aug 6, 2009 at 6:35 PM, Mark Hindess <mark.hind...@googlemail.com>wrote: > > 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 > > > -- Yours sincerely, Charles Lee
