Hi zyx, hello all,
> On 23 January 2019 at 15:39 zyx <z...@gmx.us> wrote:
> 
> 
>       Hi,
> 
> On Wed, 2019-01-23 at 13:35 +0100, Francesco Pretto wrote:
> > > weird, neither my checkout has it, nor the trunk:
> > 
> > I have ePdfDataType_Unknown = 0xff because I pushed for this change
> > some time ago[1].
> 
> I see, that's a long time ago.
> 
> > I ask you if we can move on with the change applied by Matthew, which
> > I think is wise and has the benefit of accommodating the change I was
> > aiming for.

@Francesco: Thank you.
> 
> I gave you a counter-example, where the committed change will break
> things (when the Unknown would be changed to -1 instead). For the

I wouldn't enumerate anything with a negative value, so IMO such a change
is to be -1'd (at least I would not accept non-"natural numbers" in it).
> consistency, the None is set to -1, the (other) Unknown-s to 0xff. Why
> not make them consistent in a way to declare Unknown/None as the first
> value of the enum with the value -1? Would that work for you? If not,

AFAIK, "None" and "Unknown" are different (e.g. for filters "None" means
it's known that no filter is applied whereas "Unknown" means it isn't
known which filter is applied (or the filter applied is unsupported in
PoDoFo, at least in the used revision). Therefore, where "None" makes
sense, I'm OK with (actually, would prefer) it being the first value,
but not -1 (or other negative) unless it'd be necessary to prevent zero-
initialization to yield "None" as valid value (on the contrary, I'd be
concerned if it'd yield some arbitrary, maybe unintended filter, even if
documented as the default filter, except in default construction PdfFilter()).

Another reason is that I wouldn't know, reading the source code, how many
1- (set) bits I'd get with an enum value -1, given the enum size can change
by compiler option (with 0xff any extra bits would be zeroed padding AFAICS).  

> why not? I kind of miss what you are aiming for. Is it anything else
> than having all Unknown being 0xff, which can eventually cause waste of
> memory in combination with -fshort-enums (while the switch is

In the reference Francesco mentioned in his first answer [1] to my first
post "[PATCH 1/5] is problematic" I couldn't find anything suggesting the
existence of less-than-8-bit enums, so 0xff (can be stored in 8 bit) does
waste clearly no memory, even with -fshort-enums (I didn't search all its
pages, just read the one about that option, but IIRC types shorter than 8
bit are only possible with bitfields).

> 
> > If you are reverting
> 
> I'm not going to revert anything. I just express my opinion on the
> change.
> 

@zyx: In light of what I've written above (and in pots before), would you
please allow the change making Unknown = 0xff go in?
> > I will not complain but I kindly ask you to comment the code with a
> > warning so nobody will attempt to change that underlying type anymore
> 
> Right, it makes sense to add the comment there. I'm not going to add it
> there myself though. Not yet at least.

I'm volunteering to add it, but only after the security fixes are all in,
and the Unknown has been added with positive value.

> 
>       Bye,
>       zyx

Best regards, Matthew

[1] Message-ID: 
<CALas-iiwB=FEF=1-o+bf4cejso2kyho0e-_00g2b7u7ree9...@mail.gmail.com>
    Date: Tue, 22 Jan 2019 23:16:02 +0100
    Archive URL: https://sourceforge.net/p/podofo/mailman/message/36524577/


_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to