On Oct 8, 2014, at 5:26 AM, Ștefan-Gabriel Mirea wrote:
> I've been trying to tackle the second bug in the BUGS file, namely a
> crash of g-iges in count_non_union_ops().
Cool, that bug is considerably more "complicated" as you've discovered, with
lots of issues in abundance. That used to be one of our biggest, oldest, and
most complex converters until recently and it hasn't been seriously reviewed
for conformance in many years.
> Also, there seems to be a problem with write_freeform() where a string
> is split into lines, because spaces are not regarded as separators, so
> if you have a space followed by a string (e. g. ' 4Test'), the length
> will not be identified as a number but read together with the space
> and the string as a regular field which cannot be split, which
> sometimes leads to infinite loops.
There have been many changes to our underlying utility library in the past
couple years, so it's entirely possible an issue in that converter would have
gotten overlooked as changes were made. Someone would have likely just made
sure it kept compiling without regard for correctness (that converter doesn't
have a regression test in place).
> After I modified the function by handling spaces the same way as
> commas and semicolons, the application neither crashed nor looped
> anymore, but the generated IGES file (example[1]) is not valid (I
> tried opening it with gCAD3D 2.20 and FreeCAD 0.14 without success,
> even though I can open the sample file on Wikipedia[2]).
Google Pro-Tip: you can find files of nearly any time around the web very
easily but just adding "filetype:TYPE" where TYPE is the filename suffix you
want it to report. Searching for filetype:igs and iges return a few iges files
that might be helpful reference.
There are also three sample files in src/conv/iges -- generated from three
different applications, and I believe they're even three different versions of
IGES (3, 4, and 5).
> I initially thought that it was not going to be hard to understand the
> MGED file format as long as I have the source code of both g-iges and
> iges-g, but there are some problems. For example, look at the file
> below[1] on line 26, where an empty string (the third field) is not
> displayed at all (which is explicitly set in iges.c, lines 2723 ->
> 2726), whereas iges-g (more precisely the Readname() function) can
> only recognize empty strings as "0H", otherwise crashes.
AHA... there's a major bug that invalidates the output right there on 2723 and
demonstrated in your output file. You can have empty fields, but putting a
space in the field is not the same as an empty field... (", , ," vs ",,,") The
IGES file format is very field-sensitive.
We have automatic code formatters that automatically put spaces after commas in
the source code. The code was "modernized" (looks like in 2011, commit 42149)
to clean it up and spaces were inadvertently injected within many string
literals in that converter. This needs to be undone.
Care to test if that fixes your case? Basically want to run "svn diff -c42149
src/conv/iges" and identify all the string literals that had spaces injected,
and undo them. The trick we use elsewhere that allows the beautifier to still
work is to have a #define COMMA ',' and add them to the print string with %c so
it will cause a compiler error if ',' is ever changed to ', '.
> I looked for some specifications of this standard but I didn't find
> anything complete. I'd be especially interested in a comprehensive
> reference of all the entities and their parameters. Any help would be
> appreciated.
The entire 5.3 specification is available online:
http://www.uspro.org/documents/IGES5-3_forDownload.pdf
> Also, should I submit patches with the changes I made so
> far even thought the converter still doesn't work as expected?
Conceptually different changes should be separate patches. If there's any
reason that your changes shouldn't or can't be applied, then it's not useful to
submit them as a patch just yet. If they can be implied and provide some
documentable improvement and don't knowingly introduce a new issue, definitely
submit it. ;)
Cheers!
Sean
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
BRL-CAD Developer mailing list
brlcad-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/brlcad-devel
