Re: Categories containing "++" make gnatsweb crash

[Lars Henriksen]

On Tue, Apr 29, 2003 at 09:39:54PM +0200, Yngve Svendsen wrote:
> At 15:28 20.01.2003 +0100, Dieperink Alwin wrote:
> >Hello,
> >
> >When categories contain a "++" in their name, gnatsweb crashes with the
> >following error:
> 
> (snip)
> 
> >There are several solutions:
> >1) restrict the use of special characters in category names and correct
> >"categories"
> >2) correct the use of grep in gnatsweb.
> >3) ...
> >
> >My preference goes actually to the first solution.
> 
> Thanks for reporting this. I ended up modifying the code to avoid the use 
> of grep -- it wasn't a good idea to use it anyway since it is an 
> unnecessarily expensive operation in this context. And moreover, I do not 
> want to assume anything about allowed characters in GNATS category names.

Here is the code:

Index: gnatsweb.pl
===================================================================
RCS file: /cvsroot/gnatsweb/gnatsweb/gnatsweb.pl,v
retrieving revision 1.120
retrieving revision 1.121
diff -c -r1.120 -r1.121
*** gnatsweb.pl 8 Jan 2003 13:04:17 -0000       1.120
--- gnatsweb.pl 29 Apr 2003 19:25:35 -0000      1.121

[snip]

*** 1577,1588 ****
      if (fieldinfo ($_, 'fieldtype') eq 'enum')
      {
        my $default = $fields{$_};
        # Check whether field value is a known enumeration value.
!       if (!(grep /^$default$/, @$values))
!       {
!       push(@$values, 'unknown') if (!grep /^unknown$/, @$values);
!       $default = 'unknown';
!       }
        print popup_or_scrolling_menu($_, $values, $default),
              "</td>\n</tr>\n";
      }
--- 1577,1595 ----
      if (fieldinfo ($_, 'fieldtype') eq 'enum')
      {
        my $default = $fields{$_};
+         my $found = 0;
        # Check whether field value is a known enumeration value.
!         foreach(@$values)
!         {
!               next if ($_ ne $default);
!           $found = 1;
!               last;
!         }
!         unless ($found)
!         {
!               push(@$values, 'unknown');
!               $default = 'unknown';
!         }
        print popup_or_scrolling_menu($_, $values, $default),
              "</td>\n</tr>\n";
      }

I agree that assumptions about characters should be avoided and that the
foreach-construction does this, but please lecture me: is perl-grep
particularly expensive?.

I have another comment, though. If the field value isn't a known enumeration
value, the value 'unknown' is inserted in the enumeration array and made
the default value for display. But in my original code the insertion of
'unknown' only happened if it wasn't already there (the second grep operation
above). The reason is that 'unknown' may be a perfectly valid enumeration
value (and if it is already there, we don't want to insert it a second time).
E.g. I have the following entry in the responsible file:

unknown:The name is not in the responsible file - use "view" to see 
it.:gnats-admin

If you edit a PR with an unknown responsible, the descriptive text is then
diplayed as a guide to the user.

Here is a suggestion for a modified foreach-loop:

 my $found = 0;
 my $nopush = 0;
 # Check whether field value is a known enumeration value.
 foreach(@$values)
 {
   $found = 1 if $_ eq $default;
   $nopush = 1 if $_ eq 'unknown';
 }
 unless ($found)
 {
   push(@$values, 'unknown') unless $nopush;
   $default = 'unknown';
 }

Lars Henriksen