Gerry Carter noted "Samba does not allow mixing of comment lines and parameters. Be careful not to put comments on the same line as anything else, such as: path = /data # server's data partition Errors such as this, where the parameter value is defined with a string, can be tricky to notice. The testparm program won’t complain. The only clues you’ll receive are that testparm reports the path parameter set to /d # server's data partition, and the failures that result when clients attempt to access the share." I claim this is a bug in both the parser and testparm.
A possible proof of concept is: /* * stripcommment -- remove non-quoted # comments from the * right side of text lines. The only quotes supported * are double quotes ("), as single quotes may actually * be apostrophies. As this is specifically for Samba, * strips the \n, as that's already done, instead of * reinserting it. */ #include <stdio.h> #include <stdlib.h> char * stripcomment(char *line) { int quoted = 0; char *p = line; int c; for (; (c = *p) != NULL; p++) { switch (c) { case '"': quoted = !quoted; break; case '#': if (quoted) { continue; } else { *p = '\0'; return line; } case '\n': *p = '\0'; return line; } } return line; } #ifdef TEST void main() { char line[1024]; while (fgets(line, sizeof(line), stdin) != NULL) { printf("%s\n", stripcomment(line)); } exit(0); } #endif
I'm unclear, but I think this bug was meant to be directed at Samba3. We should sync up the two different loadparm systems however.
For Samba 3, a tentative fix which parses comment = a comment "with a quoted #" and a # comment as comment = a comment "with a quoted #" and a follows: froggy> sccs diffs -u params.c ------- params.c ------- --- /tmp/sccs.e9aOPV Tue Mar 28 22:53:03 2006 +++ params.c Tue Mar 28 22:51:12 2006 @@ -119,6 +119,10 @@ /* be sure to return chars >127 as positive values */ return (int)( *(f->p++) & 0x00FF ); } +static void myungetc(myFILE *f, int c) { + f->p--; + *(f->p) = (char) (c & 0x00FF); +} static void myfile_close(myFILE *f) { @@ -347,6 +351,7 @@ int i = 0; /* Position within bufr. */ int end = 0; /* bufr[end] is current end-of-string. */ int vstart = 0; /* Starting position of the parameter value. */ + int doingQuote = False; /* Processing quoted string. */ const char *func = "params.c:Parameter() -"; /* Read the parameter name. */ @@ -422,24 +427,46 @@ } switch(c) { - case '\r': /* Explicitly remove '\r' because the older */ - c = mygetc( InFile ); /* version called fgets_slash() which also */ - break; /* removes them. */ + /* Explicitly remove '\r' because the older */ + /* version called fgets_slash() which also */ + /* removes them.*/ + case '\r': + c = mygetc( InFile ); + break; - case '\n': /* Marks end of value unless there's a '\'. */ + /* A '\n' marks end of value unless there's a '\'. */ + case '\n': i = Continuation( bufr, i ); if( i < 0 ) { c = 0; } else { - for( end = i; (end >= 0) && isspace((int)bufr[end]); end-- ) + for (end = i; (end >= 0) && isspace((int)bufr[end]); end-- ) ; c = mygetc( InFile ); } break; - default: /* All others verbatim. Note that spaces do not advance <end>. This allows trimming */ + /* An unquoted # means a comment to EOL. */ + case '#': + if (! doingQuote) { + while (mygetc( InFile ) != '\n') + ; + myungetc(InFile, '\n'); + c = '\n'; + } + goto default_label; + + case '"': + doingQuote = ! doingQuote; + /* Fall through into default case. */ + + /* All others verbatim. Note that spaces do not */ + /* advance <end>. This allows trimming of */ + /* whitespace at the end of the line. */ + default_label: /* got default is not legal... */ + default: bufr[i++] = c; - if( !isspace( c ) ) /* of whitespace at the end of the line. */ + if( !isspace( c ) ) end = i; c = mygetc( InFile ); break; @@ -487,7 +514,7 @@ case ';': /* Comment line. */ case '#': - c = EatComment( InFile ); + c = EatComment( InFile ); break; case '[': /* Section Header. */ Some of the above are comment changes so I could read them, and do not affect the fix. --dave c-b
And here's the same fix against samba 4, at revision 14828. froggy> sccs diffs -u params.c ------- params.c ------- --- /tmp/sccs.ooa4Jf Thu Mar 30 17:24:12 2006 +++ params.c Thu Mar 30 17:23:31 2006 @@ -106,6 +106,12 @@ return (int)( *(f->p++) & 0x00FF ); } +static void myungetc(myFILE *f, int c) { + f->p--; + *(f->p) = (char) (c & 0x00FF); +} + + static void myfile_close(myFILE *f) { talloc_free(f); @@ -313,6 +319,7 @@ int i = 0; /* Position within bufr. */ int end = 0; /* bufr[end] is current end-of-string. */ int vstart = 0; /* Starting position of the parameter value. */ + int doingQuote = False; /* Processing quoted string. */ const char *func = "params.c:Parameter() -"; /* Read the parameter name. */ @@ -419,9 +426,23 @@ } break; + case '#': + if (! doingQuote) { + while (mygetc( InFile ) != '\n') + ; + myungetc(InFile, '\n'); + c = '\n'; + } + goto default_label; + + case '"': + doingQuote = ! doingQuote; + /* Fall through into default case. */ + default: /* All others verbatim. Note that spaces do */ - InFile->bufr[i++] = c; /* not advance <end>. This allows trimming */ - if( !isspace( c ) ) /* of whitespace at the end of the line. */ + default_label: /* not advance <end>. This allows trimming */ + InFile->bufr[i++] = c; /* of whitespace at the end of the line. */ + if( !isspace( c ) ) end = i; c = mygetc( InFile ); break;
The following patch for Samba 3, revision 24545, implements comments on the same line as parameters: this smb.conf... [global] case sensitive = no # overrides auto workgroup = "FROGGY" server string = "test version # 3" passwd chat debug = yes returns the following from testparm... Press enter to see a dump of your service definitions [global] workgroup = FROGGY server string = test version # 3 passwd chat debug = Yes case sensitive = No ... froggy> cat params.diff --- params.old.c Fri Aug 10 09:56:55 2007 +++ params.c Sat Aug 18 15:37:38 2007 @@ -346,6 +346,7 @@ int i = 0; /* Position within bufr. */ int end = 0; /* bufr[end] is current end-of-string. */ int vstart = 0; /* Starting position of the parameter value. */ + int doingQuote = False; /* Processing quoted string. */ const char *func = "params.c:Parameter() -"; /* Read the parameter name. */ @@ -391,7 +392,6 @@ bufr[i] = '\0'; DEBUG(1,("%s Unexpected end-of-file at: %s\n", func, bufr )); return True; - default: if(isspace( c )) { /* One ' ' per whitespace region. */ @@ -436,7 +436,20 @@ } break; + case '"': + doingQuote = ! doingQuote; + c = mygetc( InFile ); + break; + + case '#': /* If unquoted, skip to end. */ + if (! doingQuote) { + c = EatComment( InFile ); + break; + } + /* Fall into default case. */ + default: /* All others verbatim. Note that spaces do not advance <end>. This allows trimming */ + default_label: /* Target for the above goto. */ bufr[i++] = c; if( !isspace( c ) ) /* of whitespace at the end of the line. */ end = i; Samba 4 and the announcement to follow, probably tomorrow.
Here's Samab 4, at revision 24545. From this smb.conf file: --- [global] workgroup = "FROGGY" # The quotes should be ignored server string = "test version # 3" --- we get this testparm output Loaded services file OK. Press enter to see a dump of your service definitions # Global parameters [global] workgroup = FROGGY server string = test version # 3 ... [IPC$] comment = IPC Service (test version # 3) path = /tmp --- and the diff is: --- --- params_4.old.c Sat Aug 18 15:47:51 2007 +++ params_4.c Sun Aug 19 09:54:26 2007 @@ -312,6 +312,7 @@ int i = 0; /* Position within bufr. */ int end = 0; /* bufr[end] is current end-of-string. */ int vstart = 0; /* Starting position of the parameter value. */ + int doingQuote = False; /* Processing quoted string. */ const char *func = "params.c:Parameter() -"; /* Read the parameter name. */ @@ -417,7 +418,18 @@ c = mygetc( InFile ); } break; + case '"': + doingQuote = ! doingQuote; + c = mygetc( InFile ); + break; + case '#': /* If unquoted, skip to end. */ + if (! doingQuote) { + c = EatComment( InFile ); + break; + } + /* Fall into default case. */ + default: /* All others verbatim. Note that spaces do */ InFile->bufr[i++] = c; /* not advance <end>. This allows trimming */ if( !isspace( c ) ) /* of whitespace at the end of the line. */