Applied Code Reading: GNU Plotutils

Robert, a UMLGraph user sent me an email describing a problem with the GNU plotutils SVG output on Firefox. I firmly believe that code reading is a lot easier than many think: one can easily fix most software problems without detailed knowledge of the underlying system. I therefore decided to practice what I preach.

Robert wrote.

"I've been playing with sequence diagrams and outputting to SVG format and noticed that the resulting SVG XML file isn't playing nicely with Firefox. After some experimenting and googling I found it is because the "stroke-dasharray='...'" entry in tags is missing commas between array items. According to misc forum posts it seems that the spec requires commas, and Firefox is pedantic about this. Diagrams look fine aside from all lines being solid.
E.g.:

<line ... stroke-dasharray="0.05 0.05"/>

should have been

<line ... stroke-dasharray="0.05, 0.05"/>

I tried mangling the "dashwid" variable of my sequence diagram to include a comma (e.g. "dashwid = '0.05,';) but couldn't avoid upsetting pic2plot's parser.
So I'm not sure where the problem lies - is it sequence.pic? pic2plot? libplot?"

I found and fixed the problem through the following steps.

1. Download the source code and unpack it.
2. Search for the offending code. For this I used grep and a short part of the erroneous XML string to search through all the distribution's files with the find and xargs idiom.

   find . -type f | xargs grep 'stroke-dashar'
   

   The results included three files: ./libplot/s_closepl.c, ./libplot/s_path.c, and ./test/plot2svg.xout. Common sense indicates that the problem is in s_path.c. Note that I didn't spend any time browsing the distribution's other 559 files (337,753 lines).
3. Edit the file and search for the specific pattern. Again, I didn't spend time browsing around the file's 560 lines, but concentrated on the offending code.

             sprintf (page->point, "stroke-dasharray=\"");
             _update_buffer (page);
             for (i = 0; i < num_dashes; i++)
               {
                 sprintf (page->point, "%.5g%s",
                          dashbuf[i],
                          i < num_dashes - 1 ? " " : "\"");
                 _update_buffer (page);
               }
   

   From the above code it's easy to see that the second sprintf in the loop is responsible for printing the list. Adding a comma in the space separator string fixes the problem.

                 sprintf (page->point, "%.5g%s",
                          dashbuf[i],
                          i < num_dashes - 1 ? ", " : "\"");
   

From the time stamps of the various files and directories I see that it took me less than two minutes to find and fix the bug. I hope that you will agree with me that none of the above three steps were rocket science, and that two minutes spent for locating and fixing a small bug is a reasonable time investment.

Comments   Toot! Tweet Share