On 8/23/18 12:01 AM, Vladimir D. Seleznev wrote:
RPMTAG_IDENTITY is calculating as digest of part of package header that
marked for identity calculation in rpmtag.h. They are supposed to not
contain irrelevant to package build tag entries.

Mathematically RPMTAG_IDENTITY value is a result of function of two
variable: a package header and an rpm utility, thus this value can
differ for same package and different version of rpm.

Signed-off-by: Vladimir D. Seleznev <vsele...@altlinux.org>
---
  lib/tagexts.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 72 insertions(+)

diff --git a/lib/tagexts.c b/lib/tagexts.c
index 61e9b702b..1467bfcd3 100644
--- a/lib/tagexts.c
+++ b/lib/tagexts.c
@@ -954,6 +954,77 @@ static int filenlinksTag(Header h, rpmtd td, 
headerGetFlags hgflags)
#include "identitymatch.C" +static int identityTag(Header h, rpmtd td, headerGetFlags hgflags)
+{
+    int rc = 1;
+    DIGEST_CTX ctx;
+    char * identity = NULL;
+
+    struct rpmtd_s ttd;
+    Header ih;
+    HeaderIterator hi;
+
+    /* Return immediatly if there is no identity marker in rpmtag.h */
+    if (!identityMatch(0)) {
+       rc = 0;
+       goto exit;
+    }
+
+    /* rpm v3 packages dont have an immutable region */
+    if (!headerGet(h, RPMTAG_HEADERIMMUTABLE, &ttd, HEADERGET_DEFAULT)) {
+       rc = 0;
+       goto exit;
+    }
+
+    ih = headerImport(ttd.data, ttd.count, HEADERIMPORT_FAST);
+    if ((hi = headerInitIterator(ih)) == NULL) {
+       rc = 0;
+       goto exit;
+    }
+
+    if ((ctx = rpmDigestInit(PGPHASHALGO_SHA256, RPMDIGEST_NONE)) == NULL) {
+       rc = 0;

That's no less than four cases setting rc to failure before jumping out. I'd suggest expecting failure to begin with, and only setting it to success one at the end. Which is a common idiom all over the codebase.

+       goto exit;
+    }
+
+    while (headerNext(hi, &ttd) && rc) {
+       if (rpmtdCount(&ttd) > 0) {

There should be no need for this, tags with zero count don't exist in headers AFAIK.

+           if (identityMatch(ttd.tag)) {
+               int i;
+               rpmtdInit(&ttd);

No need for rpmtdInit() here either, headerNext() takes care of that.

+               while ((i = rpmtdNext(&ttd)) >= 0) {
+                   rpmDigestUpdate(ctx, &ttd.tag, sizeof(ttd.tag));
+                   rpmDigestUpdate(ctx, &i, sizeof(i));
+                   if (ttd.type == RPM_STRING_ARRAY_TYPE) {
+                       for (int j = 0; j < ttd.count; j++)
+                           rpmDigestUpdate(ctx, ((char **) ttd.data)[j], 
strlen(((char **) ttd.data)[j]));
+                       continue;
+                   }
+                   rpmDigestUpdate(ctx, ttd.data, ttd.size);

I tend to agree with Jeff on this one - everything can be converted to strings after which all datatypes can be handled identically, and more simply.

+               }
+           }
+       }
+       rpmtdFreeData(&ttd);
+    }
+
+    if (!rc)
+       goto exit;
+
+    rpmDigestFinal(ctx, (void **) &identity, NULL, 1);
+    headerFree(ih);
+
+    td->data = xstrdup(identity);
+    td->type = RPM_STRING_TYPE;
+    td->count = 1;
+    td->flags = RPMTD_ALLOCED;
+
+exit:
+    headerFreeIterator(hi);
+    _free(identity);

Why strdup() of already malloced data? Just pass address of td->data directly to rpmDigestFinal() and avoid this unnecessary dance.

        - Panu -

+
+    return rc;
+}
+
  static const struct headerTagFunc_s rpmHeaderTagExtensions[] = {
      { RPMTAG_GROUP,           groupTag },
      { RPMTAG_DESCRIPTION,     descriptionTag },
@@ -992,6 +1063,7 @@ static const struct headerTagFunc_s 
rpmHeaderTagExtensions[] = {
      { RPMTAG_OBSOLETENEVRS,   obsoletenevrsTag },
      { RPMTAG_CONFLICTNEVRS,   conflictnevrsTag },
      { RPMTAG_FILENLINKS,      filenlinksTag },
+    { RPMTAG_IDENTITY,         identityTag },
      { 0,                      NULL }
  };

_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to