[Insight-developers] MetaIO Unknown meta-data bug

[Bradley Lowekamp]

Hello,

It unfortunately appears that my patch broke windows on both the ITK and VTK dashboards.

The methods "isblank" does not appear to be portable on windows. From a man page:

 isblank() conforms to POSIX.1-2001 and C99

I have submitted a patch here:

http://review.source.kitware.com/#/c/7155/

Brad

On Aug 22, 2012, at 4:41 PM, Williams, Norman K wrote:

> I went ahead and committed Bradley's suggested changes and pushed them to
> the SVN for MetaIO. I also addressed a long standing problem with the
> library as a Standalone project:  It wouldn't build all the way, because
> the test programs weren't linking to the ZLIB, whether it was the ITK or
> system ZLIB.
> 
> Bradley's right, the tests would detect if there was a complete failure in
> parsing, and nothing else.  And as far as I know, they don't test what
> happens if you feed MetaIO garbage text or a binary file.
> 
> MetaIO is great for what it is, but the parser is ad hoc and it isn't easy
> to follow what it's doing.  Furthermore I don't think there's any formal
> documentation of what language it recognizes; the only definition of
> correctness for an input is if the library reads the file, doesn't crash,
> and produces output that seems correct.
> 
> And actually all that tricksy fiddling about with spaces and tabs and
> newlines could be replaced with a  much more concise parser that uses C++
> iostream to tokenize the input.
> 
> If grant funding becomes available to support my job line for a month I'd
> be glad to give it a go, but for now it's a matter of "if no one can tell
> it's broke, no reason to fix it."
> --
> Kent Williams norman-k-williams at uiowa.edu
> 
> 
> 
> 
> 
> 
> On 8/15/12 3:29 PM, "Bradley Lowekamp" <blowekamp at mail.nih.gov> wrote:
> 
>> Hello,
>> I have pushed a topic with three commits to my github account:
>> 
>> https://github.com/blowekamp/ITK/commit/64b230bfa6f0d8c4b477704d3dd6033c27
>> 536e28
>> https://github.com/blowekamp/ITK/commit/f44e632683f787d3f566323a8dd5db6d4b
>> 76e619
>> https://github.com/blowekamp/ITK/commit/5c7281cfdc1888e907b85a86cbf779cc4f
>> 21cf27 *
>> 
>> *I am not entirely comfortable with the changes to the parsing of header
>> the header information and making the non-blank whitespace terminate the
>> string fields. There is a potential for some serious side effects there.
>> While all the ITK tests still pass as I was looking over the metaIO
>> object tests they appear to be the type which just write the data out and
>> then prints it to stdout, it does not verify that's it's correct. So that
>> does not help to instill confidence in the correctness in the above
>> change.
>> 
>> 
>> 
>> 
>> From: Stephen Aylward <stephen.aylward at kitware.com>
>> Date: Wednesday, August 15, 2012 9:57 AM
>> To: Bradley Lowekamp <blowekamp at mail.nih.gov>
>> Cc: ITK <insight-developers at itk.org>
>> Subject: Re: [Insight-developers] MetaIO Unknown meta-data bug
>> 
>> 
>> 
>> 
>> 
>> ...
>> 
>> 
>> 
>> Third - it is a bug we should fix.   My preference is to simply not
>> write out a tag if there is no associated value.   Agreed?
>> 
>> 
>> 
>> My initial commit did this in ITK. The second one does this in the metaIO
>> library and additionally prints a warning.
>> 
>> 
>> 
>> Then, if
>> a tag without a value is found, should it report an error, or just
>> continue on?   I guess continue on, but not report that tag as set.
>> Agreed?
>> 
>> 
>> 
>> 
>> I think it's more complicated and ambiguous.
>> 
>> Well with the current parsing/grammar? it is ambiguous as to if the
>> tag/field has an empty value, or if it's value is on the next line, which
>> may be the next tag/field. I am using the term empty value to indicate a
>> string of zero length as opposed to no value, which sounds more like a
>> NULL pointer or something.
>> 
>> My third patch does not let the parse "eat" newline or other non-blank
>> white spaces after the tag/field name. This may have other consequences,
>> in the parsing, so I am not entirely comfortable with it. If the value is
>> empty, we might as well just set the value to the empty string, if we are
>> able to parse it.
>> 
>> The more I think about the more I think it may be best to not change the
>> parsing, and just stop allowing the metaIO library to write ambiguous
>> empty value for fields, which is accomplished in the first two patchs.
>> Agree?
>> 
>> 
>> 
>> Thanks,
>> Stephen
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> ========================================================
>> Bradley Lowekamp
>> Medical Science and Computing for
>> Office of High Performance Computing and Communications
>> National Library of Medicine
>> blowekamp at mail.nih.gov
>> 
>> 
>> 
>> 
>> 
> 
> 
> 
> ________________________________
> Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
> ________________________________

========================================================
Bradley Lowekamp  
Medical Science and Computing for
Office of High Performance Computing and Communications
National Library of Medicine 
blowekamp at mail.nih.gov



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-developers/attachments/20120823/ddfcf1c8/attachment.htm>