[Libreoffice] [PATCH] harmonization of BOOL vs sal_Bool as a prep for BOOL to bool converstion

[Norbert Thiebaud]

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
> 	  bool-related-issues
>
> 	* 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
to
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
>> converstion
>
> 	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[1]) [ 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.
>
[....]
>
> --
>  michael.meeks at novell.com  <><, Pseudo Engineer, itinerant idiot
>
>