[Libreoffice] [PATCH] harmonization of BOOL vs sal_Bool as a prep for BOOL to bool converstion
nthiebaud at gmail.com
Mon Oct 4 09:46:25 PDT 2010
On 10/4/10, Michael Meeks <michael.meeks at novell.com> wrote:
> Hi Norbert,
> So - first ... thanks for your great set of patches, it is nice to see
> clean-ness trickling into the code :-) I reviewed, built, tested, and
> merged these patches:
> * Convert all virtual functions QueryValue() and PutValue() to
> return bool instead of a mix of BOOL and sal_Bool - ~1Mb ;-)
> * native bool support in SvXMLUnitConverter and connexe
> * harmonized some headers with implementation regarding BOOL
> vs sal_Bool use.
> Which all looked great. They all looked good. Of course, for this task
> we should be a bit careful about UNO compiled methods which use sal_Bool
> - although, I suspect that even they would cause little trouble [ but
> breaking the ABI related assembler binary bridges would be a little
> unfortunate ].
My goal is to push sal_Bool (i.e bool as unsigned char) as much as
possible to the boundary.
But I'm not sure where that ABI boundary is. Is there a document
somewhere that explain what is the ABI (IOW what interfaces are 'ABI
public' versus interface used only internally
> Anyhow - I didn't merge this:
> * Add support for bool for the operator << and >> of SvStream
> + didn't apply this one: do we really need it (yet?)
> + was concerned about accidental usage really.
The way I am proceeding is to change
typedef sal_Bool BOOL
typedef bool BOOL
and then compile, find problems, fix them, then change back the typedef
re-compile - to make sure that there are still no problem
and make a patch with the change so far
Without the support of << >> fo bool, this is the first thing that
break and I would have to
re-patch them every time to move forward (and not forget to undo it
before submitting a patch...
Eventually this will be needed anyway and in the mean time, while BOOL
is typdef'ed as sal_Bool this is without impact.
> + also, sizeof (bool) is not a good idea; if we are
> writing to a binary stream, we want a fixed size eg.
> a char on every platform, and not to change the binary
> format (of course).
Yes good point. I didn't realized then that sizeof(bool) is not
specified by the C++ standard
I can very easily keep the serializetion use sizeof(char).
> On Sun, 2010-10-03 at 14:39 -0500, Norbert Thiebaud wrote:
>> [PATCH] harmonization of BOOL vs sal_Bool as a prep for BOOL to bool
> Or this attached patch, which (unfortunately) defeated me - too much
> line wrapping grief, we should really teach your mailer about that ;-)
Yep, sorry about that. I will be more careful in the future.
> Better - we should get you direct commit access, can you go through the:
> http://freedesktop.org/wiki/AccountRequests process ?
I can... but one step will be difficult: the signing of the key.
Unless you know someone in the Dallas area that can do that for me ?
how about setting up a git repo on github or something, so that you
can pull from that rather than post-processsing my emails ?
> I'd be happy for this sort of cleanup to be committed directly, under
> three conditions:
> a) we don't touch any UNO-ised methods (yet) [ these are
> generated with sal_Bool anyway ].
Again. How do I identify for sure what constitute a UNO-ised method ?
> b) the code compiles cleanly
> c) you have checked a diff before/after of:
> 'make vtable_check'
> to ensure we didn't accidentally spike any virtual methods
What does vtable_check check (*)? How to interpret the result ?
(*) yes it check the vtables... that much I figured out :-) but bear
with me. I'm a C guy. My C++ experience is very superficial.
> Just as an example my diff -u from before or after showed some false
> positives [ a patch from Kohei, and your sw class rename to remove the
> ambiguity ] which was interesting, I append the output for your
> delectation. A correct (pure) re-factor, that doesn't include this stuff
> would of course be an empty diff.
> Anyhow - it'd be great if you could re-send your latest work without
> the line wrapping (as an attachment is perhaps best) - and an assurance
> that the vtable check passes :-)
> Many thanks,
> michael.meeks at novell.com <><, Pseudo Engineer, itinerant idiot
More information about the LibreOffice