harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Charles Lee <littlee1...@gmail.com>
Subject Re: svn commit: r801484 - HARMONY-6279 file.encoding improvement
Date Fri, 07 Aug 2009 02:16:47 GMT
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.hindess@googlemail.com>wrote:

>
> Thanks Charles.  I've made those changes.  What about the more important
> 'inline' issue?
>
> Regards,
>  Mark.
>
> In message <5948b71e0908060209g2c844f83sc2a920c1ddfe3c2f@mail.gmail.com>,
> > Charles Lee writes:
> >
> > On Thu, Aug 6, 2009 at 4:54 PM, Mark Hindess <
> mark.hindess@googlemail.com>
> > wrote:
> >
> > > Charles, Regis,
> > >
> > > Some comments regarding this commit...
> > >
> > > In message <20090806010230.9DD53238887A@eris.apache.org>,
> > > regisxu@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][.codeset][@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message