[haiku-commits] Re: r37323 - in haiku/trunk: build/jam headers/os/locale src/kits/locale src/preferences/locale
- From: PulkoMandy <pulkomandy@xxxxxxxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Thu, 1 Jul 2010 13:58:37 +0000
2010/7/1 Ingo Weinhold <ingo_weinhold@xxxxxx>
>
> Why the extra class? The GetCatalog() method could just as well be member
> of
> BLocaleRoster, which would also allow to make the conceptually private
> BLocaleRoster::GetCatalog(BCatalog*, vint32*) actually private. I would
> rather just make sCatalog a static variable in the source file (i.e. not a
> member) -- no need for anyone to see it.
>
The goal was to have a shared object-local storage, and for that I need to
link this class statically into every shared object needing localization. It
is built as a separate library named liblocalestub.a and linked in each
shared object. Is there another way to do that ? Having the catalog and the
InitOnce flag living outside of any class is possible, but I don't see what
more it gives us.
>
> And finally, the GetCatalog() method should have hidden visibility, so that
> it is not possible to accidentally forget to link against the static
> library.
> Unfortunately gcc 2 doesn't support the visibility function attribute, so
> this has to be done via inline assembly. Have a look at the
> HIDDEN_FUNCTION()
> macro in src/libs/posix_error_mapper/posix_error_mapper.h. Must be given
> the
> mangle function name.
>
I don't know much about assembly tricks :)
However, if the lib is missing the binaries will not link already, because
every call to B_TRANSLATE will try to call this GetCatalog method and find
that it's missing.
>
> [...]
> > +BCatalog*
> > +BLocaleRoster::GetCatalog(BCatalog* catalog, vint32* catalogInitStatus)
> > +{
> > + // This function is used in the translation macros, so it can't
> return
> > a
> > + // status_t. Maybe it could throw exceptions ?
>
> The method should be private. The catalog == NULL check is superfluous. In
> all other error cases returning an empty catalog is just fine, I guess.
>
ok, will do that.
An empty catalog should fallback to return the string given, so that would
be ok.
>
> GetSignature() can fail (e.g. when the signature is not set). The array
> remains uninitialized in this case and bad things will happen below.
>
Will add a check for this.
>
> Just add
>
> DEFINES += B_TRANSLATE_USE_NEW_MACROS ;
>
> in the Jamfile. Then you don't have to add the macro definition in every
> source file. Alternatively you can do that at a central place in the build
> system via the AppendToConfigVar rulle. E.g. at the end of BuildSetup,
> where
> "-Werror" is appended to C[++]FLAGS in a similar manner.
>
>
Good to know, but this was just a test, as I plan to update all apps to
usethe new system, then remove the macro.
Thanks for having a look !
--
Adrien Destugues / PulkoMandy
http://pulkomandy.ath.cx
Other related posts:
