From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on sa.local.altlinux.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=unavailable autolearn_force=no version=3.4.1 To: devel@lists.altlinux.org References: <20191206133647.dculnmwkd3yf2wjp@titan.localdomain> <20191206153655.86334-1-darktemplar@altlinux.org> <20191208232108.GC30742@altlinux.org> <20191210000737.GD15867@altlinux.org> <1ee850d3-9ebd-ae95-2665-f5ba7fb86ad4@altlinux.org> <20191210102009.GB22650@altlinux.org> <20191210222017.GA31774@altlinux.org> <20191212224315.b5b14736ba94b86fa85a830c@altlinux.org> From: Aleksei Nikiforov Message-ID: <32bfab23-32ca-9acb-d668-d1f9120646ae@altlinux.org> Date: Fri, 13 Dec 2019 11:01:47 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20191212224315.b5b14736ba94b86fa85a830c@altlinux.org> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Language: ru Content-Transfer-Encoding: 8bit Subject: Re: [devel] [PATCH for apt 2/2 v2] Fix pointer arithmetics X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Dec 2019 08:01:53 -0000 Archived-At: List-Archive: List-Post: 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 >