Re: [devel] [SCM] packages/apt: heads/rework-dynamic-mmap

[Aleksei Nikiforov <darktemplar@altlinux.org>]

Здравствуй.

29.01.2020 19:24, Ivan Zakharyaschev пишет:
> Hello!
> 
> On Wed, 29 Jan 2020, Aleksei Nikiforov wrote:
> 
>> Увидел данное изменение и по нему у меня появилось несколько вопросов.
> 
> Хорошо, что обратил внимание.
> 
>> Насколько я вижу, по большей части это rebase/cherry-pick одного из моих
>> коммитов, но с дополнительными изменениями поверх них. Почему не написать в
>> таком случае, что это изменение, сделанное Вами, и опционально указать на чём
>> оно основано, если считаете это нужным? Особенно если планируете использовать
> 
> С этим нет проблем. Я могу оформить сообщение как кому хочется.
> 
>> его в том виде, в котором он существует. Я спрашиваю этот вопрос из-за того,
>> что к дополнительным изменениям, сделанным помимо rebase и разрешения
>> возникших конфликтов, у меня есть вопросы.
> 
> Хорошо, что обратил внимание и написал. Спасибо!
> 
> Я хотел обсудить это и хотел, как мы это завели, написать в devel@ свои
> вопросы и просьбу посмотреть, прокомментировать. Сейчас как раз это и
> случится, раз обсуждение уже пошло.
> 
>> Помимо вопроса где планируется использовать данное изменение, поскольку в
> 
> Я просто уделяю внимание тем изменениям, которые были в последних релизах,
> и разбираюсь в них, навожу порядок в понимании и оформлении, чтобы у
> читающего было ясное понимание, почему эти изменения происходят. (Чиатющим
> могу быть я в будущем или кто-то другой.)
> 
> Прежде всего, отвлекаясь от вопросов, скажу, почему я его перенёс в
> истории пораньше: Обсуждение арифметики указателей ещё не закончилось (и
> прочее, связанное с динамическим realloc/mremap, сложнее понять), а это
> простое по сути изменение, которые и читаемость и понимание кода улучшает
> на мой взгляд, поэтому я его переставил в истории пораньше. (Как более
> бесспорно правильное и как облегчающее понимание кода и логики APT-а,
> которое поможет понимать дальнейшие изменения.)
> 
> Наведение порядка в истории и в голове -- это значит для меня, что
> изменение делается по одной причине (тут -- переход на optional) и не
> смешивается с другими. Другие изменение должны получить отдельную запись о
> том, почему они оправданы.
> 
>> Сизифе и p9 оно уже присутствует, в первую очередь у меня вопрос к следующему
>> куску кода:
>>
>>> diff --git a/apt/apt-pkg/rpm/rpmindexfile.cc
>> b/apt/apt-pkg/rpm/rpmindexfile.cc
>>> index 271cd8f..9a8a7f9 100644
>>> --- a/apt/apt-pkg/rpm/rpmindexfile.cc
>>> +++ b/apt/apt-pkg/rpm/rpmindexfile.cc
>>> @@ -406,8 +406,10 @@ bool rpmPkgListIndex::Merge(pkgCacheGenerator
>> &Gen,OpProgress &Prog) const
>>>          FileFd Rel(RelFile,FileFd::ReadOnly);
>>>          if (_error->PendingError() == true)
>>>    	 return false;
>>> -      Parser.LoadReleaseInfo(File,Rel);
>>> +      const bool Result = Parser.LoadReleaseInfo(File,Rel);
>>>          Rel.Seek(0);
>>> +      if (!Result)
>>> +	 return false;
>>>       }
>>>
>>>       return true;
>>
>> По сравнению с моей версией, тут не убран вызов "Rel.Seek(0)". Мне не понятна
>> причина такого изменения по сравнению с кодом, на котором основано данное
>> изменение.
> 
> Да, конечно, я это заметил. Но Rel.Seek(0) никак не связан с введением
> optional в API и я совершенно не знал, когда это увидел, почему он был
> убран. Объяснений, которые я мог бы прочитать, не нашёл.
> 
> Если это оправданное изменение, будет хорошо такой коммит с ним будет
> сделан отдельно с объяснением.
> 

Да, возможно стоило это изменение вынести в отдельный коммит.

> Спасибо за сообщение, сейчас почитаю (наверное, именно это я и хотел
> узнать):
> 
>> Если посмотреть внимательно, "FileFd Rel" объявлен несколькими
>> строками выше, и его область видимости вскоре после этой строки заканчивается,
>> а следовательно данный объект никак не используется после вызова
>> Parser.LoadReleaseInfo(...) и вскоре уничтожается. Данный вызов Rel.Seek(...)
>> - небольшая обёртка над lseek. Делать lseek перед закрытием файла смысла не
>> имеет, соответственно нет смысла и оставлять этот мёртвый код, вызов
>> Rel.Seek(...), или я что-то пропустил?
> 
> А есть идеи, почему он там раньше был? Была бессмысленная операция?
> 

Не знаю почему оно там было, но выглядит бессмысленно.

> (В любом случае, я рад любым улучшениям кода, но хотел бы очень попросить
> во всех коммитах в будущем не смешивать изменения, которые объясняются
> разными целями.)
> 
>> Вот второй фрагмент изменений под вопросом:
>>
>>> @@ -544,18 +562,24 @@ bool
>> pkgCacheGenerator::ListParser::NewProvides(pkgCache::VerIterator &Ver,
>>>    #endif
>>>
>>>       // Get a structure
>>> -   unsigned long Provides =
>> Owner->AllocateInMap(sizeof(pkgCache::Provides));
>>> -   if (Provides == 0)
>>> +   const auto Provides =
>> Owner->AllocateInMap(sizeof(pkgCache::Provides));
>>> +   const auto idxVersion = Version.empty()
>>> +                         ? std::experimental::optional<unsigned long>()
>>> +                         : WriteString(Version);
>>> +   if (!Provides || (!Version.empty() && !idxVersion))
>>>          return false;
>>>       Cache.HeaderP->ProvidesCount++;
>>>
>>>       // Fill it in
>>> -   pkgCache::PrvIterator Prv(Cache,Cache.ProvideP +
>> Provides,Cache.PkgP);
>>> +   pkgCache::PrvIterator Prv(Cache,Cache.ProvideP +
>> *Provides,Cache.PkgP);
>>>       Prv->Version = Ver.Index();
>>>       Prv->NextPkgProv = Ver->ProvidesList;
>>>       Ver->ProvidesList = Prv.Index();
>>> -   if (Version.empty() == false && (Prv->ProvideVersion =
>> WriteString(Version)) == 0)
>>> -      return false;
>>> +
>>> +   if (Version.empty() == false)
>>> +   {
>>> +      Prv->ProvideVersion = *idxVersion;
>>> +   }
>>>
>>>       // Locate the target package
>>>       pkgCache::PkgIterator Pkg;
>>
>> Здесь мне следующее условие кажется неоправданно усложнённым и трудночитаемым
>> по сравнение с кодом из моего изменения:
>>
>> if (!Provides || (!Version.empty() && !idxVersion))
>>
>> Конечно, за счёт этого в условии "if (Version.empty() == false)" код сокращён
>> заметно по сравнению с моей версией, но все те строки, хоть их было и больше,
>> были простыми. Замена их на несколько усложнённых строк - не то, что я выбрал
>> бы в данном случае. Или в данном случае я тоже пропустил какую-то причину
>> данного изменения?
> 
> Не то, чтобы есть практические причины, но вызывало удивление и вопрос в
> твоей версии, что заполняется бессмысленное значение 0 в случае, если
> довести эту "транзакцию" по выделению и заполнению памяти не удалось
> довести успешно до конца:
> 
>      // Get a structure
>      const auto Provides =
> Owner->AllocateInMap(sizeof(pkgCache::Provides));
> -   if (Provides == 0)
> +   if (!Provides)
>         return false;
>      Cache.HeaderP->ProvidesCount++;
>      
>      // Fill it in
> -   pkgCache::PrvIterator Prv(Cache,Cache.ProvideP + Provides,Cache.PkgP);
> +   pkgCache::PrvIterator Prv(Cache,Cache.ProvideP + *Provides,Cache.PkgP);
>      Dynamic<pkgCache::PrvIterator> DynPrv(Prv);
>      Prv->Version = Ver.Index();
>      Prv->NextPkgProv = Ver->ProvidesList;
>      Ver->ProvidesList = Prv.Index();
> -   if (Version.empty() == false && (Prv->ProvideVersion = WriteString(Version)) == 0)
> -      return false;
> +
> +   if (Version.empty() == false)
> +   {
> +      const auto idxVersion = WriteString(Version);
> +      if (!idxVersion)
> +      {
> +         Prv->ProvideVersion = 0;
> +         return false;
> +      }
> +
> +      Prv->ProvideVersion = *idxVersion;
> +   }
>   
> 
> Чтобы не было удивления от бессмысленного значения, я переписал так, как
> ты увидел. Не люблю операции, которые какой-то мусор присваивают, просто
> чтобы место чем-то заполнить.
> 
> В моём варианте Prv даже не конструируется и не начинает заполняться, если
> не удалось выделить память в Map под строку Version. Это же понятнее и
> лишних вопросов не вызвает: что потом с этой частично осмысленно
> заполненной, частично незаполненной структурой произойдёт и кто её в таком
> виде увидит дальше.
> 

По сути установление нуля - это лишь сохранение старого поведения. Если 
вызов WriteString(Version) возвращал ноль, то сначала этот ноль 
присваивался Prv->ProvideVersion, а уже затем проверялось значение 
Prv->ProvideVersion. С новым API в случае ошибки возвращается особое 
значение, пустой std::experimental::optional, но для сохранения старого 
поведения данной функции pkgCacheGenerator::ListParser::NewProvides ноль 
присваивается переменной Prv->ProvideVersion. Я не счёл необходимым 
сильнее менять данную функцию, по крайней мере до сих пор, но если есть 
желание дальше изменять данный код - я не возражаю.

>> 29.01.2020 4:21, Ivan Zakharyaschev пишет:
>>> Update of /people/imz/packages/apt.git
>>>
>>> Changes statistics since common ancestor `0.5.15lorg2-alt68.1-12-g297a12d'
>>> follows:
>>>    apt.spec                         |   2 +-
>>>    apt/apt-pkg/contrib/mmap.cc      |  36 ++++++---
>>>    apt/apt-pkg/contrib/mmap.h       |   9 ++-
>>>    apt/apt-pkg/pkgcachegen.cc       | 159
>>>    +++++++++++++++++++++++----------------
>>>    apt/apt-pkg/pkgcachegen.h        |  24 +++---
>>>    apt/apt-pkg/rpm/rpmindexfile.cc  |   4 +-
>>>    apt/apt-pkg/rpm/rpmlistparser.cc |  78 ++++++++++++++++---
>>>    apt/apt-pkg/rpm/rpmlistparser.h  |   3 +-
>>>    8 files changed, 207 insertions(+), 108 deletions(-)
>>>
>>> Changelog since common ancestor `0.5.15lorg2-alt68.1-12-g297a12d' follows:
>>> commit 6fc29243356a0ef1f3a93749114f3f0fc95992af
>>> Author: Aleksei Nikiforov <darktemplar@altlinux.org>
>>> Date:   Thu Jun 13 12:35:49 2019 +0300
>>>
>>>       Use special type to return allocation failure since 0 is a valid offset
>>>       value
>>>       
>>>       (cherry picked from commit 667677a7d268aaf0f84ce3bc0567c5350db20ec6)
>>>       
>>>       Rebased and improved by:
>>>       Ivan Zakharyaschev <imz@altlinux.org> and
>>>       Vladimir D. Seleznev <vseleznv@altlinux.org>.
>>>
>>> Full diff since common ancestor `0.5.15lorg2-alt68.1-12-g297a12d' follows:
>>> diff --git a/apt.spec b/apt.spec
>>> index 362179e..4b18a94 100644
>>> --- a/apt.spec
>>> +++ b/apt.spec
>>> @@ -230,7 +230,7 @@ printf '%_target_cpu\t%_target_cpu' >>
>>> buildlib/archtable
>>> %autoreconf
>>> %add_optflags
>>> %-DAPTRPM_ID=\\\"%name-%{?epoch:%epoch:}%version-%release%{?disttag::%disttag}.%_target_cpu\\\"
>>> %ifarch %e2k
>>> -%add_optflags -std=gnu++11
>>> +%add_optflags -std=c++14
>>> %endif
>>> %configure --includedir=%_includedir/apt-pkg %{subst_enable static}
>>>    diff --git a/apt/apt-pkg/contrib/mmap.cc b/apt/apt-pkg/contrib/mmap.cc
>>> index ef9e312..781bed0 100644
>>> --- a/apt/apt-pkg/contrib/mmap.cc
>>> +++ b/apt/apt-pkg/contrib/mmap.cc
>>> @@ -202,7 +202,7 @@ DynamicMMap::~DynamicMMap()
>>>    // DynamicMMap::RawAllocate - Allocate a raw chunk of unaligned space
>>>    /*{{{*/
>>>    // ---------------------------------------------------------------------
>>>    /* This allocates a block of memory aligned to the given size */
>>> -unsigned long DynamicMMap::RawAllocate(unsigned long Size,unsigned long
>>> Aln)
>>> +std::experimental::optional<unsigned long>
>>> DynamicMMap::RawAllocate(unsigned long Size,unsigned long Aln)
>>>    {
>>>       unsigned long long Result = iSize;
>>>       if (Aln != 0)
>>> @@ -214,18 +214,24 @@ unsigned long DynamicMMap::RawAllocate(unsigned long
>>> Size,unsigned long Aln)
>>>       if (Result + Size > WorkSpace)
>>>       {
>>>          _error->Error("Dynamic MMap ran out of room");
>>> -      return 0;
>>> +      return std::experimental::optional<unsigned long>();
>>>       }
>>>    -   return Result;
>>> +   return std::experimental::optional<unsigned long>(Result);
>>>    }
>>>    									/*}}}*/
>>>    // DynamicMMap::Allocate - Pooled aligned allocation
>>>    /*{{{*/
>>>    // ---------------------------------------------------------------------
>>>    /* This allocates an Item of size ItemSize so that it is aligned to its
>>>       size in the file. */
>>> -unsigned long DynamicMMap::Allocate(unsigned long ItemSize)
>>> -{
>>> +std::experimental::optional<unsigned long> DynamicMMap::Allocate(unsigned
>>> long ItemSize)
>>> +{
>>> +   if (ItemSize == 0)
>>> +   {
>>> +      _error->Error("Can't allocate an item of size zero");
>>> +      return std::experimental::optional<unsigned long>();
>>> +   }
>>> +
>>>       // Look for a matching pool entry
>>>       Pool *I;
>>>       Pool *Empty = 0;
>>> @@ -244,7 +250,7 @@ unsigned long DynamicMMap::Allocate(unsigned long
>>> ItemSize)
>>>          if (Empty == 0)
>>>          {
>>>    	 _error->Error("Ran out of allocation pools");
>>> -	 return 0;
>>> +	 return std::experimental::optional<unsigned long>();
>>>          }
>>>          
>>>          I = Empty;
>>> @@ -256,19 +262,25 @@ unsigned long DynamicMMap::Allocate(unsigned long
>>> ItemSize)
>>>       if (I->Count == 0)
>>>       {
>>>          I->Count = 20*1024/ItemSize;
>>> -      I->Start = RawAllocate(I->Count*ItemSize,ItemSize);
>>> -   }
>>> +      auto idxResult = RawAllocate(I->Count*ItemSize,ItemSize);
>>> +
>>> +      // Does the allocation failed ?
>>> +      if (!idxResult)
>>> +         return idxResult;
>>> +
>>> +      I->Start = *idxResult;
>>> +   }
>>>    
>>>       I->Count--;
>>>       unsigned long Result = I->Start;
>>>       I->Start += ItemSize;
>>> -   return Result/ItemSize;
>>> +   return std::experimental::optional<unsigned long>(Result/ItemSize);
>>>    }
>>>    									/*}}}*/
>>>    // DynamicMMap::WriteString - Write a string to the file
>>>    /*{{{*/
>>>    // ---------------------------------------------------------------------
>>>    /* Strings are not aligned to anything */
>>> -unsigned long DynamicMMap::WriteString(const char *String,
>>> +std::experimental::optional<unsigned long> DynamicMMap::WriteString(const
>>> char *String,
>>>    				       unsigned long Len)
>>>    {
>>>       unsigned long Result = iSize;
>>> @@ -276,7 +288,7 @@ unsigned long DynamicMMap::WriteString(const char
>>> *String,
>>>       if (Result + Len > WorkSpace)
>>>       {
>>>          _error->Error("Dynamic MMap ran out of room");
>>> -      return 0;
>>> +      return std::experimental::optional<unsigned long>();
>>>       }
>>>       
>>>       if (Len == (unsigned long)-1)
>>> @@ -284,6 +296,6 @@ unsigned long DynamicMMap::WriteString(const char
>>> *String,
>>>       iSize += Len + 1;
>>>       memcpy((char *)Base + Result,String,Len);
>>>       ((char *)Base)[Result + Len] = 0;
>>> -   return Result;
>>> +   return std::experimental::optional<unsigned long>(Result);
>>>    }
>>>    									/*}}}*/
>>> diff --git a/apt/apt-pkg/contrib/mmap.h b/apt/apt-pkg/contrib/mmap.h
>>> index efc5245..14df055 100644
>>> --- a/apt/apt-pkg/contrib/mmap.h
>>> +++ b/apt/apt-pkg/contrib/mmap.h
>>> @@ -29,6 +29,7 @@
>>>    #endif
>>>    
>>>    #include <string>
>>> +#include <experimental/optional>
>>>    #include <apt-pkg/fileutl.h>
>>>    
>>>    using std::string;
>>> @@ -90,10 +91,10 @@ class DynamicMMap : public MMap
>>>       public:
>>>    
>>>       // Allocation
>>> -   unsigned long RawAllocate(unsigned long Size,unsigned long Aln = 0);
>>> -   unsigned long Allocate(unsigned long ItemSize);
>>> -   unsigned long WriteString(const char *String,unsigned long Len =
>>> (unsigned long)-1);
>>> -   inline unsigned long WriteString(const string &S) {return
>>> WriteString(S.c_str(),S.length());};
>>> +   std::experimental::optional<unsigned long> RawAllocate(unsigned long
>>> Size,unsigned long Aln = 0);
>>> +   std::experimental::optional<unsigned long> Allocate(unsigned long
>>> ItemSize);
>>> +   std::experimental::optional<unsigned long> WriteString(const char
>>> *String,unsigned long Len = (unsigned long)-1);
>>> +   inline std::experimental::optional<unsigned long> WriteString(const
>>> string &S) {return WriteString(S.c_str(),S.length());};
>>>       void UsePools(Pool &P,unsigned int Count) {Pools = &P; PoolCount =
>>>       Count;};
>>>       
>>>       DynamicMMap(FileFd &F,unsigned long Flags,unsigned long WorkSpace =
>>>       2*1024*1024);
>>> diff --git a/apt/apt-pkg/pkgcachegen.cc b/apt/apt-pkg/pkgcachegen.cc
>>> index 070c0f0..ae0d4e3 100644
>>> --- a/apt/apt-pkg/pkgcachegen.cc
>>> +++ b/apt/apt-pkg/pkgcachegen.cc
>>> @@ -54,16 +54,22 @@ pkgCacheGenerator::pkgCacheGenerator(DynamicMMap
>>> *pMap,OpProgress *Prog) :
>>>    
>>>       if (Map.Size() == 0)
>>>       {
>>> +      const auto idxAlloc = Map.RawAllocate(sizeof(pkgCache::Header));
>>> +      const auto idxVerSysName = WriteStringInMap(_system->VS->Label);
>>> +      const auto idxArch =
>>> WriteStringInMap(_config->Find("APT::Architecture"));
>>> +      if ((!idxAlloc) || (!idxVerSysName) || (!idxArch))
>>> +         return;
>>> +
>>>          // Setup the map interface..
>>> -      Cache.HeaderP = (pkgCache::Header *)Map.Data();
>>> -      Map.RawAllocate(sizeof(pkgCache::Header));
>>> +      Cache.HeaderP = (pkgCache::Header *)(static_cast<char*>(Map.Data()) +
>>> *idxAlloc);
>>> +
>>>          Map.UsePools(*Cache.HeaderP->Pools,sizeof(Cache.HeaderP->Pools)/sizeof(Cache.HeaderP->Pools[0]));
>>>          
>>>          // Starting header
>>>          *Cache.HeaderP = pkgCache::Header();
>>> -      Cache.HeaderP->VerSysName = WriteStringInMap(_system->VS->Label);
>>> -      Cache.HeaderP->Architecture =
>>> WriteStringInMap(_config->Find("APT::Architecture"));
>>> -      Cache.ReMap();
>>> +      Cache.HeaderP->VerSysName = *idxVerSysName;
>>> +      Cache.HeaderP->Architecture = *idxArch;
>>> +      Cache.ReMap();
>>>       }
>>>       else
>>>       {
>>> @@ -97,17 +103,17 @@ pkgCacheGenerator::~pkgCacheGenerator()
>>>    }
>>>    									/*}}}*/
>>>    // CacheGenerator::WriteStringInMap
>>> /*{{{*/
>>> -unsigned long pkgCacheGenerator::WriteStringInMap(const char *String,
>>> +std::experimental::optional<map_ptrloc>
>>> pkgCacheGenerator::WriteStringInMap(const char *String,
>>>       					unsigned long Len) {
>>>       return Map.WriteString(String, Len);
>>>    }
>>>    									/*}}}*/
>>>    // CacheGenerator::WriteStringInMap
>>> /*{{{*/
>>> -unsigned long pkgCacheGenerator::WriteStringInMap(const char *String) {
>>> +std::experimental::optional<map_ptrloc>
>>> pkgCacheGenerator::WriteStringInMap(const char *String) {
>>>       return Map.WriteString(String);
>>>    }
>>>    									/*}}}*/
>>> -unsigned long pkgCacheGenerator::AllocateInMap(unsigned long size) {/*{{{*/
>>> +std::experimental::optional<map_ptrloc>
>>> pkgCacheGenerator::AllocateInMap(unsigned long size) {/*{{{*/
>>>       return Map.Allocate(size);
>>>    }
>>>    									/*}}}*/
>>> @@ -221,7 +227,12 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
>>>          }
>>>    
>>>          // Add a new version
>>> -      *Last = NewVersion(Ver,Version,*Last);
>>> +      const auto verindex = NewVersion(Ver,Version,*Last);
>>> +      if (!verindex)
>>> +         return _error->Error(_("Error occurred while processing %s
>>> (NewVersion0)"),
>>> +                              PackageName.c_str());
>>> +      *Last = *verindex;
>>> +
>>>          Ver->ParentPkg = Pkg.Index();
>>>          Ver->Hash = Hash;
>>>          if (List.NewVersion(Ver) == false)
>>> @@ -365,21 +376,20 @@ bool
>>> pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, const string &Nam
>>>    #endif
>>>           
>>>       // Get a structure
>>> -   unsigned long Package = AllocateInMap(sizeof(pkgCache::Package));
>>> -   if (Package == 0)
>>> +   const auto Package = AllocateInMap(sizeof(pkgCache::Package));
>>> +   const auto idxName = WriteStringInMap(Name);
>>> +   if ((!Package) || (!idxName))
>>>          return false;
>>> -
>>> -   Pkg = pkgCache::PkgIterator(Cache,Cache.PkgP + Package);
>>> -
>>> +
>>> +   Pkg = pkgCache::PkgIterator(Cache,Cache.PkgP + *Package);
>>> +
>>>       // Insert it into the hash table
>>>       unsigned long Hash = Cache.Hash(Name);
>>>       Pkg->NextPackage = Cache.HeaderP->HashTable[Hash];
>>> -   Cache.HeaderP->HashTable[Hash] = Package;
>>> -
>>> +   Cache.HeaderP->HashTable[Hash] = *Package;
>>> +
>>>       // Set the name and the ID
>>> -   Pkg->Name = WriteStringInMap(Name);
>>> -   if (Pkg->Name == 0)
>>> -      return false;
>>> +   Pkg->Name = *idxName;
>>>       Pkg->ID = Cache.HeaderP->PackageCount++;
>>>       
>>>       return true;
>>> @@ -407,7 +417,11 @@ bool
>>> pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver,
>>>          if ( (':' - '@' < 0 ? rc < 0 : rc > 0) ||
>>>               (rc != 0 && List.IsDatabase()) )
>>>             {
>>> -            Ver->VerStr = WriteStringInMap(Version);
>>> +            const auto verStrIdx = WriteStringInMap(Version);
>>> +            if (!verStrIdx)
>>> +               return false;
>>> +
>>> +            Ver->VerStr = *verStrIdx;
>>>             }
>>>       }
>>>    @@ -415,11 +429,11 @@ bool
>>> pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver,
>>>          return true;
>>>       
>>>       // Get a structure
>>> -   unsigned long VerFile = AllocateInMap(sizeof(pkgCache::VerFile));
>>> -   if (VerFile == 0)
>>> -      return 0;
>>> +   const auto VerFile = AllocateInMap(sizeof(pkgCache::VerFile));
>>> +   if (!VerFile)
>>> +      return false;
>>>       -   pkgCache::VerFileIterator VF(Cache,Cache.VerFileP + VerFile);
>>> +   pkgCache::VerFileIterator VF(Cache,Cache.VerFileP + *VerFile);
>>>       VF->File = CurrentFile - Cache.PkgFileP;
>>>       
>>>       // Link it to the end of the list
>>> @@ -441,23 +455,22 @@ bool
>>> pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver,
>>>    // CacheGenerator::NewVersion - Create a new Version
>>>    /*{{{*/
>>>    // ---------------------------------------------------------------------
>>>    /* This puts a version structure in the linked list */
>>> -unsigned long pkgCacheGenerator::NewVersion(pkgCache::VerIterator &Ver,
>>> +std::experimental::optional<unsigned long>
>>> pkgCacheGenerator::NewVersion(pkgCache::VerIterator &Ver,
>>>             const string &VerStr,
>>>             unsigned long Next)
>>>    {
>>>       // Get a structure
>>> -   unsigned long Version = AllocateInMap(sizeof(pkgCache::Version));
>>> -   if (Version == 0)
>>> -      return 0;
>>> -
>>> +   const auto Version = AllocateInMap(sizeof(pkgCache::Version));
>>> +   const auto idxVerStr = WriteStringInMap(VerStr);
>>> +   if ((!Version) || (!idxVerStr))
>>> +      return std::experimental::optional<unsigned long>();
>>> +
>>>       // Fill it in
>>> -   Ver = pkgCache::VerIterator(Cache,Cache.VerP + Version);
>>> +   Ver = pkgCache::VerIterator(Cache,Cache.VerP + *Version);
>>>       Ver->NextVer = Next;
>>>       Ver->ID = Cache.HeaderP->VersionCount++;
>>> -   Ver->VerStr = WriteStringInMap(VerStr);
>>> -   if (Ver->VerStr == 0)
>>> -      return 0;
>>> -
>>> +   Ver->VerStr = *idxVerStr;
>>> +
>>>       return Version;
>>>    }
>>>    									/*}}}*/
>>> @@ -474,12 +487,12 @@ bool
>>> pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator &Ver,
>>>       pkgCache &Cache = Owner->Cache;
>>>       
>>>       // Get a structure
>>> -   unsigned long Dependency =
>>> Owner->AllocateInMap(sizeof(pkgCache::Dependency));
>>> -   if (Dependency == 0)
>>> +   const auto Dependency =
>>> Owner->AllocateInMap(sizeof(pkgCache::Dependency));
>>> +   if (!Dependency)
>>>          return false;
>>>       
>>>       // Fill it in
>>> -   pkgCache::DepIterator Dep(Cache,Cache.DepP + Dependency);
>>> +   pkgCache::DepIterator Dep(Cache,Cache.DepP + *Dependency);
>>>       Dep->ParentVer = Ver.Index();
>>>       Dep->Type = Type;
>>>       Dep->CompareOp = Op;
>>> @@ -497,8 +510,13 @@ bool
>>> pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator &Ver,
>>>      if (I->Version != 0 && I.TargetVer() == Version)
>>>          	    Dep->Version = I->Version;*/
>>>          if (Dep->Version == 0)
>>> -	 if ((Dep->Version = WriteString(Version)) == 0)
>>> -	    return false;
>>> +      {
>>> +         const auto index = WriteString(Version);
>>> +         if (!index)
>>> +            return false;
>>> +
>>> +         Dep->Version = *index;
>>> +      }
>>>       }
>>>          
>>>       // Link it to the package
>>> @@ -544,18 +562,24 @@ bool
>>> pkgCacheGenerator::ListParser::NewProvides(pkgCache::VerIterator &Ver,
>>>    #endif
>>>       
>>>       // Get a structure
>>> -   unsigned long Provides =
>>> Owner->AllocateInMap(sizeof(pkgCache::Provides));
>>> -   if (Provides == 0)
>>> +   const auto Provides = Owner->AllocateInMap(sizeof(pkgCache::Provides));
>>> +   const auto idxVersion = Version.empty()
>>> +                         ? std::experimental::optional<unsigned long>()
>>> +                         : WriteString(Version);
>>> +   if (!Provides || (!Version.empty() && !idxVersion))
>>>          return false;
>>>       Cache.HeaderP->ProvidesCount++;
>>>       
>>>       // Fill it in
>>> -   pkgCache::PrvIterator Prv(Cache,Cache.ProvideP + Provides,Cache.PkgP);
>>> +   pkgCache::PrvIterator Prv(Cache,Cache.ProvideP + *Provides,Cache.PkgP);
>>>       Prv->Version = Ver.Index();
>>>       Prv->NextPkgProv = Ver->ProvidesList;
>>>       Ver->ProvidesList = Prv.Index();
>>> -   if (Version.empty() == false && (Prv->ProvideVersion =
>>> WriteString(Version)) == 0)
>>> -      return false;
>>> +
>>> +   if (Version.empty() == false)
>>> +   {
>>> +      Prv->ProvideVersion = *idxVersion;
>>> +   }
>>>       
>>>       // Locate the target package
>>>       pkgCache::PkgIterator Pkg;
>>> @@ -579,23 +603,24 @@ bool pkgCacheGenerator::SelectFile(const string &File,
>>> const string &Site,
>>>    				   unsigned long Flags)
>>>    {
>>>       // Get some space for the structure
>>> -   CurrentFile = Cache.PkgFileP + AllocateInMap(sizeof(*CurrentFile));
>>> -   if (CurrentFile == Cache.PkgFileP)
>>> +   const auto idxFile = AllocateInMap(sizeof(*CurrentFile));
>>> +   const auto idxFileName = WriteStringInMap(File);
>>> +   const auto idxSite = WriteUniqString(Site);
>>> +   const auto idxIndexType = WriteUniqString(Index.GetType()->Label);
>>> +   if ((!idxFile) || (!idxFileName) || (!idxSite) || (!idxIndexType))
>>>          return false;
>>> +   CurrentFile = Cache.PkgFileP + *idxFile;
>>>       
>>>       // Fill it in
>>> -   CurrentFile->FileName = WriteStringInMap(File);
>>> -   CurrentFile->Site = WriteUniqString(Site);
>>> +   CurrentFile->FileName = *idxFileName;
>>> +   CurrentFile->Site = *idxSite;
>>>       CurrentFile->NextFile = Cache.HeaderP->FileList;
>>>       CurrentFile->Flags = Flags;
>>>       CurrentFile->ID = Cache.HeaderP->PackageFileCount;
>>> -   CurrentFile->IndexType = WriteUniqString(Index.GetType()->Label);
>>> +   CurrentFile->IndexType = *idxIndexType;
>>>       PkgFileName = File;
>>>       Cache.HeaderP->FileList = CurrentFile - Cache.PkgFileP;
>>>       Cache.HeaderP->PackageFileCount++;
>>> -
>>> -   if (CurrentFile->FileName == 0)
>>> -      return false;
>>>       
>>>       if (Progress != 0)
>>>          Progress->SubProgress(Index.Size());
>>> @@ -606,7 +631,7 @@ bool pkgCacheGenerator::SelectFile(const string &File,
>>> const string &Site,
>>>    // ---------------------------------------------------------------------
>>>    /* This is used to create handles to strings. Given the same text it
>>>       always returns the same number */
>>> -unsigned long pkgCacheGenerator::WriteUniqString(const char *S,
>>> +std::experimental::optional<unsigned long>
>>> pkgCacheGenerator::WriteUniqString(const char *S,
>>>    						 unsigned int Size)
>>>    {
>>>       /* We use a very small transient hash table here, this speeds up
>>> generation
>>> @@ -614,7 +639,7 @@ unsigned long pkgCacheGenerator::WriteUniqString(const
>>> char *S,
>>>       pkgCache::StringItem *&Bucket = UniqHash[(S[0]*5 + S[1]) %
>>>       _count(UniqHash)];
>>>       if (Bucket != 0 &&
>>>           stringcmp(S,S+Size,Cache.StrP + Bucket->String) == 0)
>>> -      return Bucket->String;
>>> +      return std::experimental::optional<unsigned long>(Bucket->String);
>>>       
>>>       // Search for an insertion point
>>>       pkgCache::StringItem *I = Cache.StringItemP +
>>>       Cache.HeaderP->StringList;
>>> @@ -632,24 +657,23 @@ unsigned long pkgCacheGenerator::WriteUniqString(const
>>> char *S,
>>>       if (Res == 0)
>>>       {
>>>          Bucket = I;
>>> -      return I->String;
>>> +      return std::experimental::optional<unsigned long>(I->String);
>>>       }
>>>       
>>>       // Get a structure
>>> -   unsigned long Item = AllocateInMap(sizeof(pkgCache::StringItem));
>>> -   if (Item == 0)
>>> -      return 0;
>>> +   const auto Item = AllocateInMap(sizeof(pkgCache::StringItem));
>>> +   const auto idxString = WriteStringInMap(S, Size);
>>> +   if ((!Item) || (!idxString))
>>> +      return std::experimental::optional<unsigned long>();
>>>    
>>>       // Fill in the structure
>>> -   pkgCache::StringItem *ItemP = Cache.StringItemP + Item;
>>> +   pkgCache::StringItem *ItemP = Cache.StringItemP + *Item;
>>>       ItemP->NextItem = I - Cache.StringItemP;
>>> -   *Last = Item;
>>> -   ItemP->String = WriteStringInMap(S,Size);
>>> -   if (ItemP->String == 0)
>>> -      return 0;
>>> -
>>> +   *Last = *Item;
>>> +   ItemP->String = *idxString;
>>> +
>>>       Bucket = ItemP;
>>> -   return ItemP->String;
>>> +   return std::experimental::optional<unsigned long>(ItemP->String);
>>>    }
>>>             /*}}}*/
>>>    @@ -882,7 +906,10 @@ bool pkgMakeStatusCache(pkgSourceList
>>> &List,OpProgress &Progress,
>>>       {
>>>          // Preload the map with the source cache
>>>          FileFd SCacheF(SrcCacheFile,FileFd::ReadOnly);
>>> -      if (SCacheF.Read((unsigned char *)Map->Data() +
>>> Map->RawAllocate(SCacheF.Size()),
>>> +      const auto idxAllocate = Map->RawAllocate(SCacheF.Size());
>>> +      if (!idxAllocate)
>>> +         return false;
>>> +      if (SCacheF.Read((unsigned char *)Map->Data() + *idxAllocate,
>>>      	       SCacheF.Size()) == false)
>>>      return false;
>>>    diff --git a/apt/apt-pkg/pkgcachegen.h b/apt/apt-pkg/pkgcachegen.h
>>> index a187cd1..69c4f2e 100644
>>> --- a/apt/apt-pkg/pkgcachegen.h
>>> +++ b/apt/apt-pkg/pkgcachegen.h
>>> @@ -24,6 +24,8 @@
>>>    
>>>    #include <apt-pkg/pkgcache.h>
>>>    
>>> +#include <experimental/optional>
>>> +
>>>    class pkgSourceList;
>>>    class OpProgress;
>>>    class MMap;
>>> @@ -34,10 +36,10 @@ class pkgCacheGenerator
>>>       private:
>>>    
>>>       pkgCache::StringItem *UniqHash[26];
>>> -   unsigned long WriteStringInMap(const std::string &String) { return
>>> WriteStringInMap(String.c_str(), String.length()); };
>>> -   unsigned long WriteStringInMap(const char *String);
>>> -   unsigned long WriteStringInMap(const char *String, unsigned long Len);
>>> -   unsigned long AllocateInMap(unsigned long size);
>>> +   std::experimental::optional<unsigned long> WriteStringInMap(const
>>> std::string &String) { return WriteStringInMap(String.c_str(),
>>> String.length()); };
>>> +   std::experimental::optional<unsigned long> WriteStringInMap(const char
>>> *String);
>>> +   std::experimental::optional<unsigned long> WriteStringInMap(const char
>>> *String, unsigned long Len);
>>> +   std::experimental::optional<unsigned long> AllocateInMap(unsigned long
>>> size);
>>>    
>>>       public:
>>>       
>>> @@ -57,15 +59,15 @@ class pkgCacheGenerator
>>>       bool FoundFileDeps;
>>>       
>>>       bool NewFileVer(pkgCache::VerIterator &Ver,ListParser &List);
>>> -   unsigned long NewVersion(pkgCache::VerIterator &Ver,const string
>>> &VerStr,unsigned long Next);
>>> +   std::experimental::optional<unsigned long>
>>> NewVersion(pkgCache::VerIterator &Ver,const string &VerStr,unsigned long
>>> Next);
>>>    
>>>       public:
>>>    
>>>       // CNC:2003-02-27 - We need this in rpmListParser.
>>>       bool NewPackage(pkgCache::PkgIterator &PkgI,const string &Pkg);
>>>    -   unsigned long WriteUniqString(const char *S,unsigned int Size);
>>> -   inline unsigned long WriteUniqString(const string &S) {return
>>> WriteUniqString(S.c_str(),S.length());};
>>> +   std::experimental::optional<unsigned long> WriteUniqString(const char
>>> *S,unsigned int Size);
>>> +   inline std::experimental::optional<unsigned long> WriteUniqString(const
>>> string &S) {return WriteUniqString(S.c_str(),S.length());};
>>>    
>>>       void DropProgress() {Progress = 0;};
>>>       bool SelectFile(const string &File,const string &Site,pkgIndexFile
>>>       const &Index,
>>> @@ -101,10 +103,10 @@ class pkgCacheGenerator::ListParser
>>>       pkgCacheGenerator *Owner;
>>>       friend class pkgCacheGenerator;
>>>    -   inline unsigned long WriteUniqString(const string &S) {return
>>> Owner->WriteUniqString(S);};
>>> -   inline unsigned long WriteUniqString(const char *S,unsigned int Size)
>>> {return Owner->WriteUniqString(S,Size);};
>>> -   inline unsigned long WriteString(const string &S) {return
>>> Owner->WriteStringInMap(S);};
>>> -   inline unsigned long WriteString(const char *S,unsigned int Size)
>>> {return Owner->WriteStringInMap(S,Size);};
>>> +   inline std::experimental::optional<unsigned long> WriteUniqString(const
>>> string &S) {return Owner->WriteUniqString(S);};
>>> +   inline std::experimental::optional<unsigned long> WriteUniqString(const
>>> char *S,unsigned int Size) {return Owner->WriteUniqString(S,Size);};
>>> +   inline std::experimental::optional<unsigned long> WriteString(const
>>> string &S) {return Owner->WriteStringInMap(S);};
>>> +   inline std::experimental::optional<unsigned long> WriteString(const char
>>> *S,unsigned int Size) {return Owner->WriteStringInMap(S,Size);};
>>>       bool NewDepends(pkgCache::VerIterator &Ver, const string &Package,
>>>         const string &Version,unsigned int Op,
>>>         unsigned int Type);
>>> diff --git a/apt/apt-pkg/rpm/rpmindexfile.cc
>>> b/apt/apt-pkg/rpm/rpmindexfile.cc
>>> index 271cd8f..9a8a7f9 100644
>>> --- a/apt/apt-pkg/rpm/rpmindexfile.cc
>>> +++ b/apt/apt-pkg/rpm/rpmindexfile.cc
>>> @@ -406,8 +406,10 @@ bool rpmPkgListIndex::Merge(pkgCacheGenerator
>>> &Gen,OpProgress &Prog) const
>>>          FileFd Rel(RelFile,FileFd::ReadOnly);
>>>          if (_error->PendingError() == true)
>>>    	 return false;
>>> -      Parser.LoadReleaseInfo(File,Rel);
>>> +      const bool Result = Parser.LoadReleaseInfo(File,Rel);
>>>          Rel.Seek(0);
>>> +      if (!Result)
>>> +	 return false;
>>>       }
>>>    
>>>       return true;
>>> diff --git a/apt/apt-pkg/rpm/rpmlistparser.cc
>>> b/apt/apt-pkg/rpm/rpmlistparser.cc
>>> index 2fc4e01..7b946bf 100644
>>> --- a/apt/apt-pkg/rpm/rpmlistparser.cc
>>> +++ b/apt/apt-pkg/rpm/rpmlistparser.cc
>>> @@ -70,7 +70,7 @@ rpmListParser::~rpmListParser()
>>>    // ListParser::UniqFindTagWrite - Find the tag and write a unq string
>>>    /*{{{*/
>>>    // ---------------------------------------------------------------------
>>>    /* */
>>> -unsigned long rpmListParser::UniqFindTagWrite(int Tag)
>>> +std::experimental::optional<unsigned long>
>>> rpmListParser::UniqFindTagWrite(int Tag)
>>>    {
>>>       char *Start;
>>>       char *Stop;
>>> @@ -79,7 +79,7 @@ unsigned long rpmListParser::UniqFindTagWrite(int Tag)
>>>       void *data;
>>>       
>>>       if (headerGetEntry(header, Tag, &type, &data, &count) != 1)
>>> -      return 0;
>>> +      return std::experimental::optional<unsigned long>();
>>>       
>>>       if (type == RPM_STRING_TYPE)
>>>       {
>>> @@ -255,8 +255,13 @@ bool rpmListParser::NewVersion(pkgCache::VerIterator
>>> &Ver)
>>>    #endif
>>>       
>>>       // Parse the section
>>> -   Ver->Section = UniqFindTagWrite(RPMTAG_GROUP);
>>> -   Ver->Arch = UniqFindTagWrite(RPMTAG_ARCH);
>>> +   const auto idxSection = UniqFindTagWrite(RPMTAG_GROUP);
>>> +   const auto idxArch = UniqFindTagWrite(RPMTAG_ARCH);
>>> +   if ((!idxSection) || (!idxArch))
>>> +      return false;
>>> +
>>> +   Ver->Section = *idxSection;
>>> +   Ver->Arch = *idxArch;
>>>       
>>>       // Archive Size
>>>       Ver->Size = Handler->FileSize();
>>> @@ -296,7 +301,14 @@ bool rpmListParser::UsePackage(pkgCache::PkgIterator
>>> &Pkg,
>>>       if (SeenPackages != NULL)
>>>          (*SeenPackages)[Pkg.Name()] = true;
>>>       if (Pkg->Section == 0)
>>> -      Pkg->Section = UniqFindTagWrite(RPMTAG_GROUP);
>>> +   {
>>> +      const auto idxSection = UniqFindTagWrite(RPMTAG_GROUP);
>>> +      if (!idxSection)
>>> +         return false;
>>> +
>>> +      Pkg->Section = *idxSection;
>>> +   }
>>> +
>>>       if (_error->PendingError())
>>>           return false;
>>>       string PkgName = Pkg.Name();
>>> @@ -740,19 +752,61 @@ bool
>>> rpmListParser::LoadReleaseInfo(pkgCache::PkgFileIterator &FileI,
>>>       
>>>       const char *Start;
>>>       const char *Stop;
>>> +
>>>       if (Section.Find("Archive",Start,Stop))
>>> -       FileI->Archive = WriteUniqString(Start,Stop - Start);
>>> +   {
>>> +      const auto idx = WriteUniqString(Start, Stop - Start);
>>> +      if (!idx)
>>> +         return false;
>>> +
>>> +       FileI->Archive = *idx;
>>> +   }
>>> +
>>>       if (Section.Find("Component",Start,Stop))
>>> -       FileI->Component = WriteUniqString(Start,Stop - Start);
>>> +   {
>>> +      const auto idx = WriteUniqString(Start, Stop - Start);
>>> +      if (!idx)
>>> +         return false;
>>> +
>>> +      FileI->Component = *idx;
>>> +   }
>>> +
>>>       if (Section.Find("Version",Start,Stop))
>>> -       FileI->Version = WriteUniqString(Start,Stop - Start);
>>> +   {
>>> +      const auto idx = WriteUniqString(Start, Stop - Start);
>>> +      if (!idx)
>>> +         return false;
>>> +
>>> +      FileI->Version = *idx;
>>> +   }
>>> +
>>>       if (Section.Find("Origin",Start,Stop))
>>> -       FileI->Origin = WriteUniqString(Start,Stop - Start);
>>> +   {
>>> +      const auto idx = WriteUniqString(Start, Stop - Start);
>>> +      if (!idx)
>>> +         return false;
>>> +
>>> +      FileI->Origin = *idx;
>>> +   }
>>> +
>>>       if (Section.Find("Label",Start,Stop))
>>> -       FileI->Label = WriteUniqString(Start,Stop - Start);
>>> +   {
>>> +      const auto idx = WriteUniqString(Start, Stop - Start);
>>> +      if (!idx)
>>> +         return false;
>>> +
>>> +      FileI->Label = *idx;
>>> +   }
>>> +
>>>       if (Section.Find("Architecture",Start,Stop))
>>> -       FileI->Architecture = WriteUniqString(Start,Stop - Start);
>>> -
>>> +   {
>>> +      const auto idx = WriteUniqString(Start, Stop - Start);
>>> +      if (!idx)
>>> +         return false;
>>> +
>>> +      FileI->Architecture = *idx;
>>> +   }
>>> +
>>>       if (Section.FindFlag("NotAutomatic",FileI->Flags,
>>>           			pkgCache::Flag::NotAutomatic) == false)
>>>           _error->Warning(_("Bad NotAutomatic flag"));
>>> diff --git a/apt/apt-pkg/rpm/rpmlistparser.h
>>> b/apt/apt-pkg/rpm/rpmlistparser.h
>>> index 5c6173c..dd57f7c 100644
>>> --- a/apt/apt-pkg/rpm/rpmlistparser.h
>>> +++ b/apt/apt-pkg/rpm/rpmlistparser.h
>>> @@ -20,6 +20,7 @@
>>>    #include <map>
>>>    #include <vector>
>>>    #include <regex.h>
>>> +#include <experimental/optional>
>>>    
>>>    using namespace std;
>>>    
>>> @@ -45,7 +46,7 @@ class rpmListParser : public pkgCacheGenerator::ListParser
>>>    
>>>       bool Duplicated;
>>>       
>>> -   unsigned long UniqFindTagWrite(int Tag);
>>> +   std::experimental::optional<unsigned long> UniqFindTagWrite(int Tag);
>>>       bool ParseStatus(pkgCache::PkgIterator &Pkg,pkgCache::VerIterator
>>>       &Ver);
>>>       bool ParseDepends(pkgCache::VerIterator &Ver,
>>>           char **namel, char **verl, int32_t *flagl,
>>>
>>
>> С уважением,
>> Алексей Никифоров
>>