misc issues in psMetadataConfig

[David.Robbins@…]

Couple of minor issues in psMetadataConfig to consider:

a) User-defined types are being printed as metadata
b) 2 word names (of metadataItems such as "PS One") will get printed but cannot
be parsed properly afterwards- meaning that the output cannot be used for input.

I suggest replacing spaces on output with a marker (ie, an underscore (_)).

c) an invalid file returns an empty, but allocated psMetadata.
d) User-defined types do not get printed for overwrite=false in
psMetadataConfigRead (they do for "true").
e) Some metadata do not get printed for overwrite=false in psMetadataConfigRead.
(ie, "metadata7" in metaconf.in)
f) NULL times get printed as "TAI" and only as "TAI". I could see wanting to
print a NULL time as different types or not specifying the type.

[Paul Price]

(a) I expect you mean that when a metadata config ("mdc") file containing user
defined types (via the TYPE specifier) is read in, formating the resultant
metadata with psMetadataConfigFormat results in string without the user defined
types. This is fine --- we do not store the user types; they are merely a
convenience to the user when constructing mdc files.

(b) It is understood that metadata names may only have a single word. The
psMetadataConfigFormat function should generate an error if any name contains a
space (and then attempt to format the rest of the file before returning false).

(c) This is good and proper.

(d) Not sure what you mean; please be more explicit.

(e) Not sure what you mean; please be more explicit.

(f) If the time is NULL, then the type cannot be known. It is impossible to do
what you suggest, and I don't believe there is a strong use case.

[David.Robbins@…]

In response,

b) This is true for MetadataConfigParse but not for Format

d,e) the prototype is:

psMetadataConfigRead(psMetadata *md, unsigned int *nfail, const char

*filename, bool overwrite)

For whatever reason, when overwrite = false the user defined types (using the

keyworkd "TYPE") don't get printed, but they do when overwrite = true. The
same thing is true for (some of) the metadata containers. I've left a clear
example of this in tap_psMetadataConfig_input.c. It writes the read/parsed
metadata to mdc-overwrite_true.txt and mdc-overwrite_false.txt. If you compare
the two outputs, you can see that the user types "DD2" and "newC32" as well
as "metadata7" are missing from the false output.

f) it's not impossible, but I understand your point (was just a suggestion).

[Paul Price]

(b) Please make it so that psMetadataConfigFormat function will generate an
error if any name contains a space (and then attempt to format the rest of the
file before returning false).

(d) and (e) Please explain what you mean by "don't get printed" --- don't get
printed by what? psMetadataConfigRead doesn't print anything. What part of the
code should I be looking at?

[David.Robbins@…]

d, e) I meant that they're not getting parsed by psMetadataConfigRead (which
calls psMetadataConfigParse) and you can check this by doing a
psMetadataLookup, but it's even easier to see by printing out the resulting
metadata from psMetadataConfigRead by calling psMetadataConfigWrite. In
tap_psMetadataConfig_input I repeat the exact same procedure twice (call
psMetadataConfigRead with input file "metaconf.in" and then
psMetadataConfigWrite to output to the file(s) mentioned above, mdc-
overwrite_true(false).txt) but the first time I call it with the overwrite
parameter as false and the second as true. The outputs differ as I've
explained.

[Paul Price]

I don't know why the observed behaviour in (d) and (e) occurs. Sounds like a
bug in psMetadataConfig.c. It's seriously broken. We've observed that it won't
even parse the pslib.config file without errors. It worked a week ago, and if I
rewind to the CVS version of 9 days ago, it works fine.

Here's another bug I discovered when going through (the current CVS version)
trying to find the source of (d) and (e):

(gdb) p lineCount
$26 = 58
(gdb) p line
$27 = 0x8e3aba4 " META_NEW METADATA"

But the line above is actually on line 63, so lineCount is wrong. How this
happened, I have no idea.

These bugs need to be fixed, since the psMetadataConfig parsing and formatting
is an integral part of our operations.

I would suggest seriously considering rolling back all the changes you've made
to psMetadataConfig.c. It needs to be fixed, and fixed quickly.

[Paul Price]

Josh will work on fixing the psMetadataConfig.c problem. He has already looked
into some of the problems.

Dave, please either code tests for psMetadataConfig{Parse,Read} according to the
specs (since the code will be restructured as Josh works on it), or move on to
testing other functions.

[David.Robbins@…]

Ok, I'm moving on to other testing. However, I've noticed that the recent
changes have:

1) Stopped METADATA comments from being correctly parsed (ie, comment should be
"I am a Metadata" but instead is just "")

2) Caused every METADATA to include a # (even if no comment exists...)

ie,

f32 METADATA #

newi S32 151

END

3) Stopped user-types from being parsed (TYPE no longer works)

[jhoblitt]

1 & 2 were fixed this morning. I'm working on TYPES now. Thanks for catching
that one.

[jhoblitt]

I've taken a look the TYPE parsing. They were being parsed correctly prior to
psMetadataConfig.c:1.120 (commited after I started looking at the issue) but the
test test/types/tap_psMetadataConfig_input had one failure in it as more of the
metaconf.in file was being parsed then the test code was expecting. I modified
the ok_str() macro to produce additional diagnostic output to help illustrate
this problem. Post psMetadataConfig.c:1.120, TYPE parsing was broken because of
a duplicate statement in psMetadataAddItem(). I've fixed psMetadataAddItem()
and confirmed that TYPES are being parsed correctly but
test/types/tap_psMetadataConfig_input now seems to be failing with multiple test
failures and is leaking memory.

[David.Robbins@…]

I can take a look at the test(s) tomorrow if you'd like. As of right now, I'm
EXPECTING it to fail because I decided to wait until "construction" on
psMetadataConfig winds down a bit.

[jhoblitt]

I believe this has long since been resolved.