Re: [devel] [PATCH for apt 2/2 v2] Fix pointer arithmetics

[Andrey Savchenko <bircoph@altlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 3256 bytes --]

On Wed, 11 Dec 2019 10:50:22 +0300 Aleksei Nikiforov wrote:
> 11.12.2019 1:20, Dmitry V. Levin пишет:
> > On Tue, Dec 10, 2019 at 01:58:17PM +0300, Aleksei Nikiforov wrote:
> >> 10.12.2019 13:20, Dmitry V. Levin пишет:
> >>> On Tue, Dec 10, 2019 at 11:18:06AM +0300, Aleksei Nikiforov wrote:
> >>>> 10.12.2019 3:07, Dmitry V. Levin пишет:
> >>>>> On Mon, Dec 09, 2019 at 10:08:42AM +0300, Aleksei Nikiforov wrote:
> >>>>>> 09.12.2019 2:21, Dmitry V. Levin пишет:
> >>>>>>> On Fri, Dec 06, 2019 at 06:36:55PM +0300, Aleksei Nikiforov wrote:
> >>>>>>> [...]
> >>>>>>>> @@ -85,11 +87,11 @@ class pkgCache::PkgIterator
> >>>>>>>>         inline unsigned long long Index() const {return Pkg - Owner->PkgP;};
> >>>>>>>>         OkState State() const;
> >>>>>>>>      
> >>>>>>>> -   void ReMap(void const * const oldMap, void const * const newMap)
> >>>>>>>> +   void ReMap(void *oldMap, void *newMap)
> >>>>>>>
> >>>>>>> Is there any particular reason for stripping const here and in other
> >>>>>>> similar places?
> >>>>>>
> >>>>>> Yes, it's needed due to issues emerging from mixing const and non-const
> >>>>>> pointers with new and allegedly more proper way of calculating rebased
> >>>>>> pointers.
> >>>>>
> >>>>> Sorry, I don't find this argument convincing.
> >>>>> I have experienced no const issues in my version of this fix.
> >>>>
> >>>> Your version is using C-style casts in C++ code. Of course, I could use
> >>>> C-style casts or const_cast-s too to work around const correctness
> >>>> issues (i.e. to just hide these issues), and it'd work like your
> >>>> version. But I'd like to remind you that APT is C++ project, not a C
> >>>> project. What might be ok for C is sometimes a dirty ugly hack in modern
> >>>> C++.
> >>>
> >>> Sorry, I don't share you point of view on this matter.
> >>> Being a C++ project allows you to use C++ constructs, that's true,
> >>> but why do you think it prevents you from using C constructs when
> >>> appropriate?
> >>
> >> I didn't say that something prevents from using C constructs when
> >> appropriate. I'm saying that these constructs are not appropriate here.
> > 
> > Why do you think they are not appropriate here?
> > 
> 
> In good C++ code there is no place for const_cast.

This statement is ungrounded.

> Maybe there are some 
> exceptions to it, but they have to be justified. It doesn't matter that 
> you are hiding it behind C-style cast.

Please read some good book on C++ like [1] or at least the official
reference manual [2].

Aside from C-style cast C++ supports four casts (in their safety
order, the safest first):
const_cast
static_cast
dynamic_cast
reinterpret_cast

One can see their preference order base on how explicit C-style
cast is being intrepreted by the C++ compiler [3].

So actually the reinterpret_cast should be avoided when it is
possible to use more strict casts, because reinterpret_cast
disables all safety checks aside from constness and volatileness
one.

[1] Stanley B. Lippman, Josee Lajoie, C++ Primer. Chapter 4.14
Types Conversion.
[2] https://en.cppreference.com
[3] https://en.cppreference.com/w/cpp/language/explicit_cast

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]