When xmlNewPropInternal() operates on an attribute which already exists, an error return from xmlAddID is ignored, and the value supplied to xmlNewProp() won't match the attribute value unless it already did.

In the valid.c file looking at xmlAddID, you wind up in this section if the attribute you're attempting to add is already present - and NULL is returned:

-----
    if (xmlHashAddEntry(table, value, ret) < 0) {
#ifdef LIBXML_VALID_ENABLED
    /*
     * The id is already defined in this DTD.
     */
    if ((ctxt != NULL) && (ctxt->error != NULL)) {
        xmlErrValidNode(ctxt, attr->parent, XML_DTD_ID_REDEFINED,
                        "ID %s already defined\n",
                value, NULL, NULL);
    }
#endif /* LIBXML_VALID_ENABLED */
    xmlFreeID(ret);
    return(NULL);
    }
-----

But in xmlNewPropInternal() in tree.c, the return value is disregarded:

-----
    if ((value != NULL) && (node != NULL) &&
        (xmlIsID(node->doc, node, cur) == 1))
        xmlAddID(NULL, node->doc, value, cur);

    if ((__xmlRegisterCallbacks) && (xmlRegisterNodeDefaultValue))
        xmlRegisterNodeDefaultValue((xmlNodePtr) cur);
    return (cur);
}
-----

So the attribute may not match what was given to xmlNewProp(), but xmlNewProp() never finds out about it. While it's true that one should use "SetProp" to change existing attributes, it seems like a NewProp call that does not result in the attribute value being set to what was supplied should be flagged in some way.

Or at least we should clarify the xmlNewProp documentation a bit to make it clear that in order to "create-or-update" an attribute, you have to NewProp, then check the value, then SetProp if it doesn't match - or some variant of that.