|Anonymous | Login | Signup for a new account||2020-02-20 01:37 PST|
|Main | My View | View Issues|
|Viewing Issue Simple Details|
|ID||Category||Severity||Reproducibility||Date Submitted||Last Update|
|0000052||[litestep core] core||major||always||2008-06-02 11:17||2009-03-05 14:42|
|Summary||0000052: LCReadNextCommand/Config/Line implementations are broken.|
LCReadNextLine() should itterate over each and every line in the RC file.
LCReadNextCommand() should itterate over each and every line *NOT* starting with a reserved character - specifically the * character.
LCReadNextConfig() should itterate over each and every line starting with the * character that matches the pszConfig parameter.
All three functions should return the entire unparsed line including the key name.
I believe the last time this was implemented correctly was in 0.24.5 (or 11-23-99 build) before the rewrite into COM implementation. 0.24.6+ lost this functionality.
Our current SDK documentation correctly documents:
And incorrectly documents:
LCReadNextCommand - it says the function is deprecated, and does not specify that it skips lines that begin with an * or any reserved character.
|Tags||No tags attached.|
I've updated the issue with more detail. The main functionality missing from the code is that LCReadNextCommand does not skip lines that LCReadNextConfig retrieves. Basically meaning that LCReadNextCommand should skip all lines beginning with an * character. Also, LCReadNextConfig currently does not enforce that pszConfig begin with a * character. It should.
Also, the documentation for these functions state that they should return the entire line, unparsed. However we currently expand $evar$ expressions. Also, the 11-23-99 source code also expanded $evar$ expressions. So, while I think the documentation should be the correct behavior, it seems that it is not, and the documentation should be changed in that regards.
At this point there likely are many modules that depend on the current behavior. If we need the different behavior we should add a new function.
By the way, the major offender wrt multiple setting lines not starting with a * character is the core with its "LoadModule".
In my testing I've only found a few modules that are using LCReadNextConfig() incorrectly. So, fixing LCReadNextConfig() would force those modules to conform to expected conventions (ie. multiple settings of the same name must have a '*'), but as such it would cause those modules to diverge from their documentation. However, it would not "break" the modules, since with the fix in place the offending setting would simply need to be prefixed with a '*' in the user's config; it would break existing users configurations (ie. the user configurations would have to be updated to place a '*' in front of the offending settings).
Fixing LCReadNextCommand does not break any module that I'm aware of.
As such I've committed changes to CVS HEAD that fix LCReadNextCommand, and conditionally fix LCReadNextConfig (see buildoptions.h new define: LS_COMPAT_LCREADNEXTCONFIG)
I've also added support for using *LSLoadModule rather than the traditional LoadModule, this also is controlled via buildoptions.h with LS_COMPAT_LSLOADMODULE. However, LoadModule would still work even with LCReadNextConfig fix enabled. We should probably change this to support both *LSLoadModule and LoadModule at the same time for a transition period before forcing *LSLoadModule.
I am not marking this issue as resolved yet, because I think this warrants further discussion and testing to ensure these changes do not trully break prevelant modules. If it appears that this does break things beyond repair, these changes can obvsiouly be undone.
|2008-06-02 11:17||jugg||New Issue|
|2008-06-17 08:51||jugg||Note Added: 0000129|
|2008-06-17 08:51||jugg||Status||@10@ => @30@|
|2008-06-17 08:51||jugg||Description Updated|
|2008-06-17 08:51||jugg||Steps to Reproduce Updated|
|2008-06-17 08:51||jugg||Additional Information Updated|
|2008-08-18 10:52||jugg||Relationship added||child of 0000042|
|2009-01-16 16:22||ilmcuts||Status||@30@ => feedback|
|2009-02-23 22:21||jugg||Status||feedback => confirmed|
|2009-02-28 05:29||ilmcuts||Note Added: 0000242|
|2009-03-05 00:34||jugg||Note Added: 0000248|
|2009-03-05 14:41||ilmcuts||Relationship deleted||child of 0000042|
|2009-03-05 14:42||ilmcuts||Relationship added||child of 0000092|
|Copyright © 2000 - 2009 Mantis Group|