libpcp_import: address coderabbit review findings

pmiPutAtomValueHandle: validate atom is non-NULL before passing to
_pmi_stuff_atomvalue, returning PM_ERR_ARG to avoid a segfault on
PM_TYPE_STRING metrics where atom->cp would be dereferenced.

archive.c _pmi_do_desc: when a descriptor for this PMID already exists
in the archive, verify it is compatible before skipping the write.
A package upgrade may have corrected an incorrect type, semantics,
instance domain or units for a metric, in which case the old archive
cannot be extended with new data.  Return PM_ERR_LOGCHANGETYPE,
PM_ERR_LOGCHANGESEM, PM_ERR_LOGCHANGEINDOM or PM_ERR_LOGCHANGEUNITS
as appropriate so the caller has a precise diagnosis.

qa/group: add local group to qa/1670 so the standard local sweep
picks up the new append-mode coverage.

Signed-off-by: Nathan Scott <nathans@redhat.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Author: natoscott

man/man3/pmistart.3

@@ -146,6 +146,22 @@ If the archive files do not yet exist,
 .B PMI_APPEND
 behaves identically to creating a new archive (no flags).
 .PP
+.B PMI_APPEND
+and
+.B PMI_INHERIT
+may be used together.
+The inherited metric and instance-domain definitions are carried into
+the new context, and when the archive is opened on the first
+.BR pmiWrite (3)
+call, each inherited descriptor is checked against what is already
+on disk: compatible descriptors are silently skipped (no duplicate
+written); incompatible ones are rejected with an appropriate error
+(e.g.
+.BR PM_ERR_LOGCHANGETYPE ).
+When the previous context was writing to the same archive, every
+inherited descriptor already exists on disk so the combination is
+effectively a no-op beyond convenience.
+.PP
 If
 .I flags
 is zero, or neither

src/libpcp_import/src/archive.c

@@ -195,11 +195,22 @@ check_metric(pmi_context *current, pmID pmid, int *needti)
 	    /*
 	     * If the archive was opened for appending, __pmLogLoadMeta() will
 	     * have populated hashpmid with all descriptors already on disk.
-	     * Skip re-writing a descriptor that is already there — metric
-	     * descriptors are immutable, so there is no need for duplicates
-	     * that would cause .meta to grow on every short-lived invocation.
+	     * Skip re-writing a descriptor that is already there, but first
+	     * verify it is compatible with the registered metric — e.g. a
+	     * package upgrade may have corrected a metric's type, semantics,
+	     * instance domain or units, in which case the old archive cannot
+	     * be extended with the new data.
 	     */
 	    if (__pmLogLookupDesc(acp, pmid, &existing) == 0) {
+		if (existing.type != current->metric[m].desc.type)
+		    return PM_ERR_LOGCHANGETYPE;
+		if (existing.sem != current->metric[m].desc.sem)
+		    return PM_ERR_LOGCHANGESEM;
+		if (existing.indom != current->metric[m].desc.indom)
+		    return PM_ERR_LOGCHANGEINDOM;
+		if (memcmp(&existing.units, &current->metric[m].desc.units,
+			   sizeof(existing.units)) != 0)
+		    return PM_ERR_LOGCHANGEUNITS;
 		current->metric[m].meta_done = 1;
 	    }
 	    else {

src/libpcp_import/src/import.c

@@ -351,6 +351,17 @@ pmiStart(const char *archive, int flags)
     memset((void *)&current->logctl, 0, sizeof(current->logctl));
     memset((void *)&current->archctl, 0, sizeof(current->archctl));
     __pmLogWriterInit(&current->archctl, &current->logctl);
+    /*
+     * PMI_APPEND and PMI_INHERIT may be used together: the inherited
+     * metric and indom definitions are carried into the new context, and
+     * when the append archive is opened on the first pmiWrite() call,
+     * each inherited descriptor is checked against what is already on
+     * disk.  Compatible descriptors are silently skipped (no duplicate
+     * written); incompatible ones are rejected with PM_ERR_LOGCHANGE*.
+     * In the common case -- inheriting from a context that was writing
+     * to the same archive -- every inherited descriptor already exists
+     * and the combination is effectively a no-op beyond convenience.
+     */
     if ((flags & PMI_INHERIT) && old_current != NULL) {
 	current->nmetric = old_current->nmetric;
 	if (old_current->metric != NULL) {
@@ -895,6 +906,8 @@ pmiPutAtomValueHandle(int handle, pmAtomValue *atom)
 	return PM_ERR_NOCONTEXT;
     if (handle <= 0 || handle > current->nhandle)
 	return current->last_sts = PMI_ERR_BADHANDLE;
+    if (atom == NULL)
+	return current->last_sts = PM_ERR_ARG;
 
     return current->last_sts = _pmi_stuff_atomvalue(current, &current->handle[handle-1], atom);
 }