Further streamline & sanitize lead handling
authorPanu Matilainen <Panu Matilainen pmatilai@redhat.com>
Thu, 7 Jul 2011 08:55:28 +0000 (11:55 +0300)
committerPanu Matilainen <Panu Matilainen pmatilai@redhat.com>
Thu, 7 Jul 2011 08:55:28 +0000 (11:55 +0300)
- Never log anything from rpmLeadRead(), instead return an error message
  the callers can log if they see fit
- Add a return value for the lead type (which is the only bit of
  info from the lead we sometimes resort to using)
- Permit NULL pointers on all return values
- Eliminate rpmLeadCheck() and rpmLeadType() from the internal API,
  these are now combined into rpmLeadRead().
- Fix up the callers: only (re)signing needs the actual lead,
  signature verification only cares if its valid or not and
  package reading only wants the type from the lead (annoying but...)

lib/package.c
lib/rpmchecksig.c
lib/rpmlead.c
lib/rpmlead.h
sign/rpmgensig.c

index 6c16cf1..c892bab 100644 (file)
@@ -546,12 +546,11 @@ static rpmRC rpmpkgRead(rpmKeyring keyring, rpmVSFlags vsflags,
     pgpDig dig = NULL;
     char buf[8*BUFSIZ];
     ssize_t count;
-    rpmlead l = NULL;
     Header sigh = NULL;
     rpmTagVal sigtag;
     struct rpmtd_s sigtd;
     Header h = NULL;
-    char * msg;
+    char * msg = NULL;
     rpmRC rc = RPMRC_FAIL;     /* assume failure */
     int leadtype = -1;
     headerGetFlags hgeflags = HEADERGET_DEFAULT;
@@ -561,20 +560,15 @@ static rpmRC rpmpkgRead(rpmKeyring keyring, rpmVSFlags vsflags,
 
     rpmtdReset(&sigtd);
 
-    if ((rc = rpmLeadRead(fd, &l)) == RPMRC_OK) {
-       const char * err = NULL;
-       if ((rc = rpmLeadCheck(l, &err)) == RPMRC_FAIL) {
-           rpmlog(RPMLOG_ERR, "%s: %s\n", fn, err);
-       }
-       leadtype = rpmLeadType(l);
-       l = rpmLeadFree(l);
-    }
-
-    if (rc != RPMRC_OK)
+    if ((rc = rpmLeadRead(fd, NULL, &leadtype, &msg)) != RPMRC_OK) {
+       /* Avoid message spew on manifests */
+       if (rc != RPMRC_NOTFOUND) 
+           rpmlog(RPMLOG_ERR, "%s: %s\n", fn, msg);
+       free(msg);
        goto exit;
+    }
 
     /* Read the signature header. */
-    msg = NULL;
     rc = rpmReadSignature(fd, &sigh, RPMSIGTYPE_HEADERSIG, &msg);
     switch (rc) {
     default:
index 6927308..5b42a85 100644 (file)
@@ -268,7 +268,6 @@ static int rpmpkgVerifySigs(rpmKeyring keyring, rpmQueryFlags flags,
     pgpDigParams sigp;
     Header sigh = NULL;
     HeaderIterator hi = NULL;
-    rpmlead lead = NULL;
     char * msg = NULL;
     int res = 1; /* assume failure */
     rpmRC rc;
@@ -278,15 +277,9 @@ static int rpmpkgVerifySigs(rpmKeyring keyring, rpmQueryFlags flags,
     rpmDigestBundle plbundle = rpmDigestBundleNew();
     rpmDigestBundle hdrbundle = rpmDigestBundleNew();
 
-    if ((rc = rpmLeadRead(fd, &lead)) == RPMRC_OK) {
-       const char *lmsg = NULL;
-       rc = rpmLeadCheck(lead, &lmsg);
-       if (rc != RPMRC_OK) 
-           rpmlog(RPMLOG_ERR, "%s: %s\n", fn, lmsg);
-       lead = rpmLeadFree(lead);
-    }
-
-    if (rc != RPMRC_OK) {
+    if ((rc = rpmLeadRead(fd, NULL, NULL, &msg)) != RPMRC_OK) {
+       rpmlog(RPMLOG_ERR, "%s: %s\n", fn, msg);
+       free(msg);
        goto exit;
     }
 
index ad47722..195c5f7 100644 (file)
@@ -93,36 +93,36 @@ rpmRC rpmLeadWrite(FD_t fd, rpmlead lead)
     return rc;
 }
 
-rpmRC rpmLeadCheck(rpmlead lead, const char **msg)
+static rpmRC rpmLeadCheck(rpmlead lead, char **msg)
 {
     if (memcmp(lead->magic, lead_magic, sizeof(lead_magic))) {
-       if (msg) *msg = _("not an rpm package");
+       *msg = xstrdup(_("not an rpm package"));
        return RPMRC_NOTFOUND;
     }
     if (lead->signature_type != RPMSIGTYPE_HEADERSIG) {
-       if (msg) *msg = _("illegal signature type");
+       *msg = xstrdup(_("illegal signature type"));
        return RPMRC_FAIL;
     }
     if (lead->major < 3 || lead->major > 4) {
-       if (msg) *msg = _("unsupported RPM package version");
+       *msg = xstrdup(_("unsupported RPM package version"));
        return RPMRC_FAIL;
     }
     return RPMRC_OK;
 }
 
-rpmRC rpmLeadRead(FD_t fd, rpmlead *lead)
+rpmRC rpmLeadRead(FD_t fd, rpmlead *lead, int *type, char **emsg)
 {
     rpmRC rc = RPMRC_OK;
     struct rpmlead_s l;
+    char *err = NULL;
 
     memset(&l, 0, sizeof(l));
     if (Fread(&l, 1, sizeof(l), fd) != sizeof(l)) {
        if (Ferror(fd)) {
-           rpmlog(RPMLOG_ERR, _("read failed: %s (%d)\n"),
-                       Fstrerror(fd), errno);
+           rasprintf(&err, _("read failed: %s (%d)\n"), Fstrerror(fd), errno);
            rc = RPMRC_FAIL;
        } else {
-           rpmlog(RPMLOG_ERR, _("not an rpm package\n"));
+           err = xstrdup(_("not an rpm package\n"));
            rc = RPMRC_NOTFOUND;
        }
     } else {
@@ -130,17 +130,22 @@ rpmRC rpmLeadRead(FD_t fd, rpmlead *lead)
        l.archnum = ntohs(l.archnum);
        l.osnum = ntohs(l.osnum);
        l.signature_type = ntohs(l.signature_type);
+       rc = rpmLeadCheck(&l, &err);
     }
 
-    if (lead != NULL && rc == RPMRC_OK) {
-       *lead = xmalloc(sizeof(l));
-       memcpy(*lead, &l, sizeof(l));
+    if (rc == RPMRC_OK) {
+       if (lead != NULL) {
+           *lead = xmalloc(sizeof(l));
+           memcpy(*lead, &l, sizeof(l));
+       }
+       if (type != NULL)
+           *type = l.type;
+    } else {
+       if (emsg != NULL)
+           *emsg = err;
+       else
+           free(err);
     }
 
     return rc;
 }
-
-int rpmLeadType(rpmlead lead)
-{
-    return lead ? lead->type : -1;
-}
index 1854bd4..a7f4730 100644 (file)
@@ -47,26 +47,11 @@ rpmRC rpmLeadWrite(FD_t fd, rpmlead lead);
  * Read lead from file handle.
  * @param fd           file handle
  * @retval lead                pointer to package lead (malloced)
+ * @retval type                RPMLEAD_BINARY or RPMLEAD_SOURCE on success
+ * @retval emsg                failure message on error (malloced)
  * @return             RPMRC_OK on success, RPMRC_FAIL/RPMRC_NOTFOUND on error
  */
-rpmRC rpmLeadRead(FD_t fd, rpmlead *lead);
-
-/** \ingroup lead
- * Check lead for compatibility.
- * @param lead         Pointer to lead handle
- * @retval fn          Pointer to error message, NULL on success
- * @return             RPMRC_OK on success, 
- *                     RPMRC_NOTFOUND if not an rpm,
- *                     RPMRC_FAIL on invalid/incompatible rpm
- */
-rpmRC rpmLeadCheck(rpmlead lead, const char **msg);
-
-/** \ingroup lead
- * Returen type (source vs binary) of lead
- * @param lead         Pointer to lead handle
- * @return             RPMLEAD_BINARY or RPMLEAD_SOURCE, -1 on invalid
- */
-int rpmLeadType(rpmlead lead);
+rpmRC rpmLeadRead(FD_t fd, rpmlead *lead, int *type, char **emsg);
 
 #ifdef __cplusplus
 }
index 6b33254..cd61b7a 100644 (file)
@@ -433,7 +433,7 @@ static int rpmSign(const char *rpm, int deleting, const char *passPhrase)
     rpmlead lead = NULL;
     char *sigtarget = NULL, *trpm = NULL;
     Header sigh = NULL;
-    char * msg;
+    char * msg = NULL;
     int res = -1; /* assume failure */
     rpmRC rc;
     struct rpmtd_s utd;
@@ -443,18 +443,12 @@ static int rpmSign(const char *rpm, int deleting, const char *passPhrase)
     if (manageFile(&fd, rpm, O_RDONLY))
        goto exit;
 
-    if ((rc = rpmLeadRead(fd, &lead)) == RPMRC_OK) {
-       const char *lmsg = NULL;
-       rc = rpmLeadCheck(lead, &lmsg);
-       if (rc != RPMRC_OK) 
-           rpmlog(RPMLOG_ERR, "%s: %s\n", rpm, lmsg);
-    }
-
-    if (rc != RPMRC_OK) {
+    if ((rc = rpmLeadRead(fd, &lead, NULL, &msg)) != RPMRC_OK) {
+       rpmlog(RPMLOG_ERR, "%s: %s\n", rpm, msg);
+       free(msg);
        goto exit;
     }
 
-    msg = NULL;
     rc = rpmReadSignature(fd, &sigh, RPMSIGTYPE_HEADERSIG, &msg);
     switch (rc) {
     default: