Re: monitrc erroneous line numbering off by one

[Jan-Henrik Haukeland]

Christian Hopp <address@hidden> writes:

> On Thu, 12 Feb 2004, Christian Hopp wrote:
>
>> On Thu, 12 Feb 2004, Jan-Henrik Haukeland wrote:
>>
>> > Martin Pala <address@hidden> writes:
>> >
>> > >> 2.) parser increments the line position (lineno) by all empty and
>> > >> comment lines following the invalid line, which is reported. Can
>> > >> some flex guru look on it? Both attached configurations still
>> > >> produces this kind of error.
>> > >>
>> > >
>> > > This problem remains. Is here some way how to ignore "\n" for empty
>> > > and comment lines following invalid statement line?
>> >
>> > It's tricky because when the statement is treated in the action block
>> > of the parser, flex has already scanned to the next token, and if
>> > there was white space or comments "#.." the lineno variable will be
>> > increased (at least that is the hypothesis). In other words when we
>> > report an error in lineno, the actual line number can/will be off by
>> > the number of whitespace lines to the next valid token. Hmm, food for
>> > thoughts.. how to fix this whiteout hacking to much. Christian, any
>> > ideas?
>>
>> This problem happens as soon as we deal with strings (same for paths,...)
>> and statements with variable arguments... like allow.
>
> Okay its anykind of "argument" we are referring to... also numbers!

I have thought about this a bit more, the reason we have this problem
is mainly that we do not have a statement terminator. This may
typically be a problem in a "multi-part" statement where an optional
statement list is involved. Take for instance this statement:

           set httpd port 2812 address localhost 
                allow localhost
                allow blabla
           ^
           off by these lines
           v
           check ...

Because the allow list is optional Bison (the parser) needs to scan to
the next token, that is, 'check' in this example, to see if more
'allow' statement are used. This is okay from a parser perspective,
*but* the problem arise when we test the host name in a bison action,
because when we report an error the lineno variable we print will be
increased by the number of lines '\n' down to the 'check' token.

All this is rather technical, but to summarize; the problem occurs
when we test and find a semantical error in an action and refeer to
the line number. Since there was no syntax errors in the config file,
the parser scan to the next token and the lexer increment the line
number.

I have hacked the lexer whichs seems to fix the problem but the patch
is really too ugly to consider[1]. Unless you (Christian) already have
found a better way to solve this I suggest that we put this problem on
our TODO list, but at the bottom, at least for the moment.


Index: l.l
===================================================================
RCS file: /cvsroot/monit/monit/l.l,v
retrieving revision 1.71
diff -u -r1.71 l.l
--- l.l 6 Feb 2004 20:02:20 -0000       1.71
+++ l.l 12 Feb 2004 16:39:40 -0000
@@ -53,13 +53,17 @@
 
 #define COMPAT_YY_MAX_DEPTH 10
 
+#define        RET(i) do { if(lineno>trace_lineno) prev_lineno= trace_lineno;  
\
+    trace_lineno= lineno;  return(i);} while(0);
+
   int compat_yy_stack_ptr=0;
   YY_BUFFER_STATE compat_yy_stack[COMPAT_YY_MAX_DEPTH];
   void compat_yypush_buffer_state(YY_BUFFER_STATE);
   void compat_yypop_buffer_state(void);
   
   int lineno= 1;
-
+  int trace_lineno= 1;
+  int prev_lineno= 1;
   
   /* Prototypes */
   void yyerror(const char*);
@@ -115,157 +119,157 @@
 usage             {/* EMPTY */}
 was               {/* EMPTY */}
 
-{startarg}        { BEGIN(ARGUMENT_COND); return START; }
-{stoparg}         { BEGIN(ARGUMENT_COND); return STOP; }
-{execarg}         { BEGIN(ARGUMENT_COND); return EXEC; }
-
-if                { return IF; }
-then              { return THEN; }
-failed            { return FAILED; }
-ssl               { return HTTPDSSL; }
-enable            { return ENABLE; }
-disable           { return DISABLE; }
-set               { return SET; }
-daemon            { return DAEMON; }
-logfile           { return LOGFILE; }
-syslog            { return SYSLOG; }
-facility          { return FACILITY; }
-httpd             { return HTTPD; }
-address           { return ADDRESS; }
-clientpemfile     { return CLIENTPEMFILE; }
-allowselfcertification  { return ALLOWSELFCERTIFICATION; }
-certmd5           { return CERTMD5; }
-pemfile           { return PEMFILE; }
-init              { return INIT; }
-allow             { return ALLOW; }
-read[-]?only      { return READONLY; }
-pidfile           { return PIDFILE; }
-statefile         { return STATEFILE; }
-path              { return PATHTOK; }
-start             { return START; }
-stop              { return STOP; }
-port(number)?     { return PORT; }
-unix(socket)?     { return UNIXSOCKET; }
-type              { return TYPE; }
-proto(col)?       { return PROTOCOL; }
-tcp               { return TCP; }
-tcpssl            { return TCPSSL; }
-udp               { return UDP; }
-alert             { return ALERT; }
-mail-format       { return MAILFORMAT; }
-resource          { return RESOURCE; }
-restart(s)?       { return RESTART; }
-cycle(s)?         { return CYCLE;}
-timeout           { return TIMEOUT; }
-checksum          { return CHECKSUM; }
-mailserver        { return MAILSERVER; }
-every             { return EVERY; }
-host              { return HOST; }
-default           { return DEFAULT; }
-http              { return HTTP; }
-ftp               { return FTP; }
-smtp              { return SMTP; }
-pop               { return POP; }
-imap              { return IMAP; }
-nntp              { return NNTP; }
-ssh               { return SSH; }
-dwp               { return DWP; }
-ldap2             { return LDAP2; }
-ldap3             { return LDAP3; }
-rdate             { return RDATE; }
-rsync             { return RSYNC; }
-mode              { return MODE; }
-active            { return ACTIVE; }
-passive           { return PASSIVE; }
-manual            { return MANUAL; }
-group             { return GROUP; }
-uid               { return UID; }
-gid               { return GID; }
-request           { return REQUEST; }
-cpuusage          { return CPUUSAGE; }
-memusage          { return MEMUSAGE; }
-memkbyte          { return MEMKBYTE; }
-mem(ory)?         { return MEMORY; }
-totalmem(ory)?    { return TOTALMEMORY; }
-cpu               { return CPU; }
-child(ren)        { return CHILDREN; }
-timestamp         { return TIMESTAMP; }
-changed           { return CHANGED; }
-second(s)?        { return SECOND; }
-minute(s)?        { return MINUTE; }
-hour(s)?          { return HOUR; }
-day(s)?           { return DAY; }
-sslv2             { return SSLV2; }
-sslv3             { return SSLV3; }
-tlsv1             { return TLSV1; }
-sslauto           { return SSLAUTO; }
-inode(s)?         { return INODE; }
-space             { return SPACE; }
-perm(ission)?     { return PERMISSION; }
-exec(ute)?        { return EXEC; }
-size              { return SIZE; }
-connection        { return CONNECTION; }
-unmonitor         { return UNMONITOR; }
-icmp              { return ICMP; }
-echo              { return ICMPECHO; }
-send              { return SEND; }
-expect            { return EXPECT; }
-cleartext         { return CLEARTEXT; }
-md5               { return MD5TOKEN; }
-crypt             { return CRYPT; }
-signature         { return SIGNATURE; }
+{startarg}        { BEGIN(ARGUMENT_COND); RET(START); }
+{stoparg}         { BEGIN(ARGUMENT_COND); RET(STOP); }
+{execarg}         { BEGIN(ARGUMENT_COND); RET(EXEC); }
+
+if                { RET(IF); }
+then              { RET(THEN); }
+failed            { RET(FAILED); }
+ssl               { RET(HTTPDSSL); }
+enable            { RET(ENABLE); }
+disable           { RET(DISABLE); }
+set               { RET(SET); }
+daemon            { RET(DAEMON); }
+logfile           { RET(LOGFILE); }
+syslog            { RET(SYSLOG); }
+facility          { RET(FACILITY); }
+httpd             { RET(HTTPD); }
+address           { RET(ADDRESS); }
+clientpemfile     { RET(CLIENTPEMFILE); }
+allowselfcertification  { RET( ALLOWSELFCERTIFICATION); }
+certmd5           { RET(CERTMD5); }
+pemfile           { RET(PEMFILE); }
+init              { RET(INIT); }
+allow             { RET(ALLOW); }
+read[-]?only      { RET(READONLY); }
+pidfile           { RET(PIDFILE); }
+statefile         { RET(STATEFILE); }
+path              { RET(PATHTOK); }
+start             { RET(START); }
+stop              { RET(STOP); }
+port(number)?     { RET(PORT); }
+unix(socket)?     { RET(UNIXSOCKET); }
+type              { RET(TYPE); }
+proto(col)?       { RET(PROTOCOL); }
+tcp               { RET(TCP); }
+tcpssl            { RET(TCPSSL); }
+udp               { RET(UDP); }
+alert             { RET(ALERT); }
+mail-format       { RET(MAILFORMAT); }
+resource          { RET(RESOURCE); }
+restart(s)?       { RET(RESTART); }
+cycle(s)?         { RET(CYCLE);}
+timeout           { RET(TIMEOUT); }
+checksum          { RET(CHECKSUM); }
+mailserver        { RET(MAILSERVER); }
+every             { RET(EVERY); }
+host              { RET(HOST); }
+default           { RET(DEFAULT); }
+http              { RET(HTTP); }
+ftp               { RET(FTP); }
+smtp              { RET(SMTP); }
+pop               { RET(POP); }
+imap              { RET(IMAP); }
+nntp              { RET(NNTP); }
+ssh               { RET(SSH); }
+dwp               { RET(DWP); }
+ldap2             { RET(LDAP2); }
+ldap3             { RET(LDAP3); }
+rdate             { RET(RDATE); }
+rsync             { RET(RSYNC); }
+mode              { RET(MODE); }
+active            { RET(ACTIVE); }
+passive           { RET(PASSIVE); }
+manual            { RET(MANUAL); }
+group             { RET(GROUP); }
+uid               { RET(UID); }
+gid               { RET(GID); }
+request           { RET(REQUEST); }
+cpuusage          { RET(CPUUSAGE); }
+memusage          { RET(MEMUSAGE); }
+memkbyte          { RET(MEMKBYTE); }
+mem(ory)?         { RET(MEMORY); }
+totalmem(ory)?    { RET(TOTALMEMORY); }
+cpu               { RET(CPU); }
+child(ren)        { RET(CHILDREN); }
+timestamp         { RET(TIMESTAMP); }
+changed           { RET(CHANGED); }
+second(s)?        { RET(SECOND); }
+minute(s)?        { RET(MINUTE); }
+hour(s)?          { RET(HOUR); }
+day(s)?           { RET(DAY); }
+sslv2             { RET(SSLV2); }
+sslv3             { RET(SSLV3); }
+tlsv1             { RET(TLSV1); }
+sslauto           { RET(SSLAUTO); }
+inode(s)?         { RET(INODE); }
+space             { RET(SPACE); }
+perm(ission)?     { RET(PERMISSION); }
+exec(ute)?        { RET(EXEC); }
+size              { RET(SIZE); }
+connection        { RET(CONNECTION); }
+unmonitor         { RET(UNMONITOR); }
+icmp              { RET(ICMP); }
+echo              { RET(ICMPECHO); }
+send              { RET(SEND); }
+expect            { RET(EXPECT); }
+cleartext         { RET(CLEARTEXT); }
+md5               { RET(MD5TOKEN); }
+crypt             { RET(CRYPT); }
+signature         { RET(SIGNATURE); }
 include           { BEGIN(INCLUDE); }
 
-{byte}            { return BYTE; }
-{kilobyte}        { return KILOBYTE; }
-{megabyte}        { return MEGABYTE; }
-{gigabyte}        { return GIGABYTE; }
-
-{loadavg1}        { return LOADAVG1; }
-{loadavg5}        { return LOADAVG5; }
-{loadavg15}       { return LOADAVG15; }
-
-{greater}         { return GREATER; }
-{less}            { return LESS; }
-{equal}           { return EQUAL; }
-{notequal}        { return NOTEQUAL; }
+{byte}            { RET(BYTE); }
+{kilobyte}        { RET(KILOBYTE); }
+{megabyte}        { RET(MEGABYTE); }
+{gigabyte}        { RET(GIGABYTE); }
+
+{loadavg1}        { RET(LOADAVG1); }
+{loadavg5}        { RET(LOADAVG5); }
+{loadavg15}       { RET(LOADAVG15); }
+
+{greater}         { RET(GREATER); }
+{less}            { RET(LESS); }
+{equal}           { RET(EQUAL); }
+{notequal}        { RET(NOTEQUAL); }
 
 depend(s)?[ \t]+(on[ \t]*)?  {
                     BEGIN(DEPEND_COND);
-                    return DEPENDS;
+                    RET(DEPENDS);
                   } 
 
 check[ \t]+(process[ \t])? {
                     BEGIN(SERVICE_COND);
-                    return CHECKPROC;
+                    RET(CHECKPROC);
                   }
 
 check[ \t]+device {
                     BEGIN(SERVICE_COND);
-                    return CHECKDEV;
+                    RET(CHECKDEV);
                   }
 
 check[ \t]+file   {
                     BEGIN(SERVICE_COND);
-                    return CHECKFILE;
+                    RET(CHECKFILE);
                   }
 
 check[ \t]+directory {
                     BEGIN(SERVICE_COND);
-                    return CHECKDIR;
+                    RET(CHECKDIR);
                   }
 
 check[ \t]+host   {
                     BEGIN(SERVICE_COND);
-                    return CHECKHOST;
+                    RET(CHECKHOST);
                   }
 
 {number}          {
-                    yylval.number= atoi(yytext); return NUMBER;
+                   yylval.number= atoi(yytext); RET(NUMBER);
                   }
 
 {real}            {
-                    yylval.real= atof(yytext); return REAL;
+                    yylval.real= atof(yytext); RET(REAL);
                   }
 
 {real}{percent}   {
@@ -274,73 +278,73 @@
                           prog, yytext);
                       exit(1);
                     }
-                    return PERCENT;
+                    RET(PERCENT);
                   }
 
 [a-zA-Z0-9]{str}  {
                     yylval.string= xstrdup(yytext);
-                    return STRING;
+                    RET(STRING);
                   }
 
 \"[/][^\"\n]*\"   {
                     yylval.string= handle_quoted_string(yytext);
-                    return PATH;
+                    RET(PATH);
                   }
 
 \'[/][^\'\n]*\'   {
                     yylval.string= handle_quoted_string(yytext);
-                    return PATH;
+                    RET(PATH);
                   }
 
 \"[^\"]*\"        {
                     steplinenobycr(yytext);
                     yylval.string= handle_quoted_string(yytext);
-                    return STRING;
+                    RET(STRING);
                   }
 
 \'[^\']*\'        {
                     steplinenobycr(yytext);
                     yylval.string= handle_quoted_string(yytext);
-                    return STRING;
+                    RET(STRING);
                   }
 
 address@hidden     {
                     yylval.string= xstrdup(yytext);
-                    return MAILADDR;
+                    RET(MAILADDR);
                   }
 
 [/]{str}          {
                      yylval.string= xstrdup(yytext);
-                     return PATH;
+                     RET(PATH);
                   }
 
 "/"               {
                      yylval.string= xstrdup(yytext);
-                     return PATH;
+                     RET(PATH);
                   }
 
 "from:"[ address@hidden {
                       char *p= yytext+strlen("from:");
                       yylval.string = trim(xstrdup(p));
-                      return MAILFROM;
+                      RET(MAILFROM);
                   }
                      
 "subject:"[^}\n]* {
                       char *p= yytext+strlen("subject:");
                       yylval.string = trim(xstrdup(p));
-                      return MAILSUBJECT;
+                      RET(MAILSUBJECT);
                   }
 
 "message:"[^}]*   {
                       char *p= yytext+strlen("message:");
                       steplinenobycr(yytext);
                       yylval.string = trim(xstrdup(p));
-                      return MAILBODY;
+                      RET(MAILBODY);
                   }
 
 {hostname}        {
                       yylval.string = xstrdup(yytext);
-                      return STRING;
+                      RET(STRING);
                   }
 
 [\"\']            {
@@ -360,19 +364,19 @@
   {str}           {
                     yylval.string= xstrdup(yytext);
                     BEGIN(INITIAL);
-                    return SERVICENAME;
+                    RET(SERVICENAME);
                   }
 
   \"{str}\"       {
                     yylval.string= handle_quoted_string(yytext);
                     BEGIN(INITIAL);
-                    return SERVICENAME;
+                    RET(SERVICENAME);
                   }
 
   \'{str}\'       {
                     yylval.string= handle_quoted_string(yytext);
                     BEGIN(INITIAL);
-                    return SERVICENAME;
+                    RET(SERVICENAME);
                   }
 
   [\"\']          {
@@ -391,7 +395,7 @@
 
   {str}           {
                     yylval.string= xstrdup(yytext);
-                    return SERVICENAME;
+                    RET(SERVICENAME);
                   }
 
   [ \n\t]+[^,]    {
@@ -417,7 +421,7 @@
   \'[^\']*\'      {
                       steplinenobycr(yytext);
                       yylval.string= handle_quoted_string(yytext);
-                      return STRING;
+                      RET(STRING);
                   }
 
   \'              {
@@ -426,13 +430,13 @@
 
   [^ \t\n\"]+     {
                       yylval.string= xstrdup(yytext);
-                      return STRING;
+                      RET(STRING);
                   }
 
 }
 
 <INITIAL,ARGUMENT_COND,SERVICE_COND,DEPEND_COND>. {
-                      return yytext[0];
+                     RET(yytext[0]);
                   }  
 
 
Index: p.y
===================================================================
RCS file: /cvsroot/monit/monit/p.y,v
retrieving revision 1.160
diff -u -r1.160 p.y
--- p.y 11 Feb 2004 21:43:31 -0000      1.160
+++ p.y 12 Feb 2004 16:39:42 -0000
@@ -200,6 +200,7 @@
   int yylex(void);
   extern FILE *yyin;
   extern int lineno;
+  extern int prev_lineno;
   extern char *yytext;
 
   
@@ -304,7 +305,7 @@
 %token TIMEOUT RESTART CHECKSUM EXPECT EVERY 
 %token DEFAULT HTTP FTP SMTP POP IMAP NNTP SSH DWP LDAP2 LDAP3 RDATE RSYNC
 %token <string> STRING PATH MAILADDR MAILFROM MAILSUBJECT 
-%token <string> MAILBODY SERVICENAME
+%token <string> MAILBODY SERVICENAME ALLOWVALUE
 %token <number> NUMBER PERCENT
 %token <real> REAL
 %token CHECKPROC CHECKDEV CHECKFILE CHECKDIR CHECKHOST
@@ -626,7 +627,7 @@
                 | ALLOW STRING {
                    if(!add_host_allow($2)) {
                      log("%s: error: Hostname '%s' did not resolve"
-                         " at line %d\n",  prog, $2, lineno-1);
+                         " at line %d\n",  prog, $2, prev_lineno);
                      cfg_errflag++;
                    }
                    FREE($2);

-- 
Jan-Henrik Haukeland


[1] Actually, when thinking about it, the best solution is probably to
hack the parser instead and save the line number in every rule so we
can back-track and get the line number from the previous "real"
token. But this solution is not much better and involve lots of code.