psMetadata logic errors

[eugene]

There are some problems in psMetadata with the logic for adding MULTI entries,
especially when coupled to the REPLACE vs DUPLICATE flags. I've made a bunch of
edits of psMetadataAddItem and psMetadataItemAllocV to clear this up, or at
least to make it behave as I expect. I did not do a great job on error
catching, so that needs some work. I'm attaching the modified psMetadata.c and
my notes.

[eugene]

modified psMetadata.c

[eugene]

notes on metadata modifications

[robert.desonia@…]

Your code seems to have memory leaks; I'll comb through the changes you made
tomorrow.

[robert.desonia@…]

You stated:

• if the existing entry is MULTI, the new entry is added as a node of MULTI
  • it does not make sense for both the new and old entries to be of type

MULTI

• allow replacement of the existing multi with new multi (not implemented)
• if both are MULTI && !REPLACE: error

I don't think it is correct that it does not make sense for both the new and
old entries to be of type MULTI; it seemed to me that that meant that the
entries in the incoming MULTI should be merged into the existing MULTI node.
You proposed changes removed that feature.

Allowing replacement of a MULTI really doesn't make much sense to me. MULTI is
just a container for metadata items with the name; there is no MULTI in the
metadata->list because it allows entries of the same name/key. Allowing
replacement of a MULTI as if a simple metadata type makes no sense in
metadata->list representation -- the metadata is not MULTI there!

Not allowing MULTI to be added by the user is an option, but I thought it would
be handy to take all items of a specific key from one metadata, which is
returned as MULTI, and add it to another. In that case, I'd expect that to be
equivalent to adding each entry in the MULTI list individually into the new
list.

-rdd

[eugene]

I agree: my notes are still a bit confused, and don't quite match what is in new
SDRS, either.

The case in question is what do we do if both the new and existing entries are
of type MULTI. I have been thinking that (as I note for psMetadataItemAllocV)
we should require a new MULTI to contain no data (data = NULL). Adding a MULTI
item to a metadata is essentially a message saying, "build me a new MULTI"
structure. Thus, psMetadataItemAllocV should give an error if the data value is
not NULL.

Your proposal to allow a well-constructed MULTI (ie, one from an existing
psMetadata) to be passed is possible, but seems dangerous. Do we want to put
the code in psMetadataAllocV to validate that the incoming data structure is
correctly constructed as a MULTI? That means checking that the entry is a list
of psMetadataItems, all with the same names. I'm not sure it is worth the
savings. the alternative is to require the owner of the returned list to
iterate over the elements and add them by hand with psMetadataAddItem.

My simple-minded version would allow only these options:

• if the new entry is not MULTI, it should be simply added to the existing MULTI
• if the new entry *is* MULTI, the value must be NULL. In this case (both are

MULTI, new one has NULL), then no action is required since the effect of the
message ("build me a MULTI with this name") has already been done. it should
NOT be an error, contrary to what I said in my notes.

[eugene]

I believe the changes resulting from this discussion have been folded into the
SDRS and in cycle 6