Author Topic: CC doxygen processing: potential bug?  (Read 5836 times)

Offline Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
CC doxygen processing: potential bug?
« on: October 24, 2013, 05:24:04 am »
In reading through, I noticed the following:
Code: Text
  1. wxString DocumentationHelper::DoxygenToHTML(const wxString& doc)
  2. {
  3.     using namespace HTMLTags;
  4.  
  5.     wxString arguments[5];
  6.     wxString &plainText = arguments[0], brief = arguments[1], params = arguments[2],
  7.         seeAlso = arguments[3], returns = arguments[4];
  8.  
  9. [...]
  10.  
  11.     // process nested keywords:
  12.     for (size_t i = 0; i < (size_t)(sizeof(arguments)/sizeof(arguments[0])); ++i)
  13.     {
  14.         arguments[i].Trim(true).Trim(false);
  15.  
  16.         Doxygen::DoxygenParser dox_parser;
  17.         int dox_keyword = dox_parser.FindNextKeyword(arguments[i]);
  18.         while (dox_keyword < Doxygen::KEYWORDS_COUNT)
  19.         {
  20.             using namespace Doxygen;
  21.             switch (dox_keyword)
  22.             {
  23.             case B:
  24.                 {
  25.                     dox_parser.ReplaceCurrentKeyword(arguments[i], b1);
  26.                     wxString arg0;
  27.                     dox_parser.GetArgument(arguments[i], RANGE_WORD, arg0);
  28.                     arguments[i].insert(dox_parser.GetPosition() + 1, b0);
  29.                 }
  30.                 break;
  31.             default:
  32.                 break;
  33.             }
  34.             dox_keyword = dox_parser.FindNextKeyword(arguments[i]);
  35.         }
  36.     }// for (i)
  37.  
  38. [...]
  39.  
Only plainText is declared as a reference, so the following loop (the only place where arguments[] is used) does not appear to do any useful work.

However, I am not sure of the intent of this code, so I do not know what type of bug to even try to look for.  Thoughts?

ToApolytoXaos

  • Guest
Re: CC doxygen processing: potential bug?
« Reply #1 on: October 24, 2013, 07:24:06 am »
In reading through, I noticed the following:
Code: Text
  1. wxString DocumentationHelper::DoxygenToHTML(const wxString& doc)
  2. {
  3.     using namespace HTMLTags;
  4.  
  5.     wxString arguments[5];
  6.     wxString &plainText = arguments[0], brief = arguments[1], params = arguments[2],
  7.         seeAlso = arguments[3], returns = arguments[4];
  8.  
  9. [...]
  10.  
  11.     // process nested keywords:
  12.     for (size_t i = 0; i < (size_t)(sizeof(arguments)/sizeof(arguments[0])); ++i)
  13.     {
  14.         arguments[i].Trim(true).Trim(false);
  15.  
  16.         Doxygen::DoxygenParser dox_parser;
  17.         int dox_keyword = dox_parser.FindNextKeyword(arguments[i]);
  18.         while (dox_keyword < Doxygen::KEYWORDS_COUNT)
  19.         {
  20.             using namespace Doxygen;
  21.             switch (dox_keyword)
  22.             {
  23.             case B:
  24.                 {
  25.                     dox_parser.ReplaceCurrentKeyword(arguments[i], b1);
  26.                     wxString arg0;
  27.                     dox_parser.GetArgument(arguments[i], RANGE_WORD, arg0);
  28.                     arguments[i].insert(dox_parser.GetPosition() + 1, b0);
  29.                 }
  30.                 break;
  31.             default:
  32.                 break;
  33.             }
  34.             dox_keyword = dox_parser.FindNextKeyword(arguments[i]);
  35.         }
  36.     }// for (i)
  37.  
  38. [...]
  39.  
Only plainText is declared as a reference, so the following loop (the only place where arguments[] is used) does not appear to do any useful work.

However, I am not sure of the intent of this code, so I do not know what type of bug to even try to look for.  Thoughts?
I don't know why Alpha, but I have a feeling that the code's creator original intention was to create something like a series of variables of type wxString &. It resembles so much to the <type specifier> * issue most C developers have with pointers. Maybe the poor guy was too tired when he was creating this piece of code and missed the original concept (intention)? Possible and plausible for sure; it happens often to each and every one of us ;)

Offline p2rkw

  • Almost regular
  • **
  • Posts: 142
Re: CC doxygen processing: potential bug?
« Reply #2 on: October 24, 2013, 08:40:05 pm »
Hi, it seems that I'm author of this code (doxygen 'parser' and popup) ;) My first idea was to increase readability: use array inside loop and reference afterwards. Lets say 'named array elements'. In fact ampersands are missing.

Offline Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: CC doxygen processing: potential bug?
« Reply #3 on: October 25, 2013, 09:41:34 pm »
So because the loop has technically not been used, and no problems were noticed from that, can the array and the loop be completely removed?  Or is there a case in which this produces incorrect results?

Offline p2rkw

  • Almost regular
  • **
  • Posts: 142
Re: CC doxygen processing: potential bug?
« Reply #4 on: October 25, 2013, 11:37:11 pm »
Now I see I should write more comments.
First loop (while) looks for keywords that affect whole paragraph, and (should) store these paragraphs inside array "arguments", then second loop (for) looks for nested keywords inside spitted paragraphs. So this is definitely a bug. Paragraphs should be accessible in both ways.
Try to run parser on this, to see a bug:
Code: [Select]
/*! Detailed description starts here.
  \b Bold is now @b supported
  @sa this will not be @b bold
*/
int a;

Offline Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: CC doxygen processing: potential bug?
« Reply #5 on: October 26, 2013, 11:17:13 pm »
Committed.