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> From: Aleksei Nikiforov Message-ID: Date: Mon, 9 Dec 2019 10:08:42 +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: <20191208232108.GC30742@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: Mon, 09 Dec 2019 07:09:55 -0000 Archived-At: List-Archive: List-Post: 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. > [...] >> @@ -301,7 +302,7 @@ std::experimental::optional DynamicMMap::Allocate(unsigned long Item >> Pool* oldPools = Pools; >> auto idxResult = RawAllocate(I->Count*ItemSize,ItemSize); >> if (Pools != oldPools) >> - I += Pools - oldPools; >> + I = RebasePointer(I, oldPools, Pools); >> >> // Does the allocation failed ? >> if (!idxResult) > > In my patch RebasePointer invocation was after the idxResult check, > not before the check. > Theoretically, order here might be important. In practice, it doesn't matter. > By the way, in this and other similar cases, > is there any reason for "Pools != oldPools" check? > Is RebasePointer incapable of handling this, or is it an optimization? > It's just an optimization, it may be removed. > [...] >> diff --git a/apt/apt-pkg/rebase_pointer.h b/apt/apt-pkg/rebase_pointer.h >> new file mode 100644 >> index 0000000..f6b3c15 >> --- /dev/null >> +++ b/apt/apt-pkg/rebase_pointer.h >> @@ -0,0 +1,16 @@ >> +#ifndef PKGLIB_REBASE_POINTER_H >> +#define PKGLIB_REBASE_POINTER_H >> + >> +template >> +static inline T* RebasePointer(T *ptr, void *old_base, void *new_base) >> +{ >> + return reinterpret_cast(reinterpret_cast(new_base) + (reinterpret_cast(ptr) - reinterpret_cast(old_base))); >> +} >> + >> +template >> +static inline const T* RebasePointer(const T *ptr, void *old_base, void *new_base) >> +{ >> + return reinterpret_cast(reinterpret_cast(new_base) + (reinterpret_cast(ptr) - reinterpret_cast(old_base))); >> +} >> + >> +#endif > > Do we really need two templates here? > > Yes, second template with const ptr is needed for rpmListParser::rpmListParser from rpmlistparser.cc. Variable SeenPackages has type SeenPackagesType, which is a typedef to std::set. Thus, elements are 'const char*', and either it should be const-casted to 'char*', which is ugly, or const-correctness should be achieved some other way, for example by getting rid of unimportant const qualifiers like in my changes. And first template is needed for every other case with non-const ptr. > > _______________________________________________ > Devel mailing list > Devel@lists.altlinux.org > https://lists.altlinux.org/mailman/listinfo/devel >