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