On Wed, May 02, 2012 at 05:23:30PM +0200, David Kalnischkies wrote:
> > 2129.4.17 kalnisc | /* Record the Description (it is not translated) */
> > | MD5SumValue CurMd5 = List.Description_md5();
> > | if (CurMd5.Value().empty() == true)
> > | return true;
> > | std::string CurLang = List.DescriptionLanguage();
> > |
> > | /* Before we add a new description we first search in the group for
> > | a version with a description of the same MD5 - if so we reuse this
> > | description group instead of creating our own for this version */
> > | for (pkgCache::PkgIterator P = Grp.PackageList();
> > | P.end() == false; P = Grp.NextPkg(P))
> > | {
> > | for (pkgCache::VerIterator V = P.VersionList();
> > | V.end() == false; ++V)
> > | {
> > | if (IsDuplicateDescription(V.DescriptionList(), CurMd5, "") == false)
> > | continue;
> > | Ver->DescriptionList = V->DescriptionList;
> > | return true;
> > | }
> > | }
> >
> > When IsDuplicateDescription is called, calling md5() on the V.DescriptionList()
> > points to unallocated memory. Any idea why that could be?
>
> So you mean the Description struct is invalid (V.DescriptionList()) or
> the V.DescriptionList().md5() char* ?
The latter does not point to any initialized memory.
> We are missing a bit of error checking here (callers of NewDescription() do
> not check if return is != 0 and IsDuplicateDescription doesn't check if the
> given Description is valid), but both shouldn't be a problem as NewDescription
> can only really fail if new memory can't be allocated and as each version has
> at least one description you shouldn't hit a problem in the dup check either.
> Both wouldn't be limited to s390x either way:
> We seem to have a similar bugreport from ppc64 (#669243),
> if i understand right it's also bigendian 64bit, but no other report.
Yes. So would be sparc64. s390x is the only in-archive arch that's 64bit be.
> The code in pkgcachegen.cc was reworked for multi-arch and especially the dup
> check is new and the code as such works wih pointer left and right, but non of
> it should be architecture dependent… Somehow i fear that it's more related to
> our checksum changes. We had way to many problems with sha1 and sha2 to assume
> md5 would be okay (the code for md5 itself is not new, but the code warping
> around it).
At least the problem was not in constructing the checksum object, but in
retrieving the content for it (i.e. md5() in IsDuplicateDescription).
> You mentioned bisecting? Any insigns which revisions are (not) effected?
> (bzr has no bisect included by default, and last time i check the plugin was…
> lets say suboptimal for us as we tend to have "big" merges)
Either the plugin was broken or the tree it acted upon was useless to bisect.
I don't know.
Kind regards
Philipp Kern
Attachment:
signature.asc
Description: Digital signature