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

[Aleksei Nikiforov <darktemplar@altlinux.org>]

12.12.2019 22:43, Andrey Savchenko пишет:
> 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.
> 

It is grounded. Read my answer below (and links [2] and [3] to 
documentation you provided).

>> 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].

Same to you.

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

Now, this statement is ungrounded. Where did you get this list from? Do 
you serously think that dynamic_cast is less safe than static_cast? And 
how did you judge safety of const_cast vs safety of reinterpret_cast? 
How did you get const_cast more safe than static_cast and dynamic_cast?

For your education: const_cast is indeed safe when 'T' is casted to 
'const T', but when 'const T' is casted to 'T' it can lead to UB, 
assuming 'T' is a non-const type.

Read your own documentation:

https://en.cppreference.com/w/cpp/language/const_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.
> 

Please show me how you'd write this code without reinterpret_cast and 
without C-style cast since it will just implicitly use reinterpret_cast. 
C-style cast is the least strict cast in C++ since it tries to apply 
every cast except for dynamic_cast. I agree with you that stricter casts 
should be applied, and thus there's no place for C-style casts.

> [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
> 
> 
> _______________________________________________
> Devel mailing list
> Devel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel
>