Hi, well after some ever further investigation of some bugs, I ended up cleaning up the code, again. I also fixed some really heavy memory leaks in csvToList and made it in general more stable. Regards, Patrick "Marex" Niklaus -------------- next part -------------- A non-text attachment was scrubbed... Name: ini.c.patch Type: text/x-patch Size: 8275 bytes Desc: not available Url : http://lists.freedesktop.org/archives/compiz/attachments/20070413/bf740b2d/ini.c.bin
Patrick Niklaus wrote:> Hi, > > well after some ever further investigation of some bugs, I ended up > cleaning up the code, again. I also fixed some really heavy memory > leaks in csvToList and made it in general more stable.Thanks very much, is it possible that you can split the patch into separate ones so that if there is a problem with the general fixes then we could revert those without losing the readability changes or the memory leak fixes? Its very easy if you are using git. Just make each change and then use git-commit -a. When you have finished with all the changes type 'git format-patch origin' and it will dump out a load of files beginning with 000*. If you send those, then I can apply each one more easily. I think it will be easier for you too. See the bottom of this page for more information (sorry the link was lost in the changes) http://www.compiz.org/index.php?title=Develop
Hi, ok I hope this patches are now ok for you to commit. Regards, Patrick Niklaus -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Fixed-list-parsing-plugged-a-memory-leak.patch Type: text/x-patch Size: 2586 bytes Desc: not available Url : http://lists.freedesktop.org/archives/compiz/attachments/20070413/ad78562a/0001-Fixed-list-parsing-plugged-a-memory-leak-0001.bin -------------- next part -------------- A non-text attachment was scrubbed... Name: 0002-Cleaned-up-iniParseLine.patch Type: text/x-patch Size: 1672 bytes Desc: not available Url : http://lists.freedesktop.org/archives/compiz/attachments/20070413/ad78562a/0002-Cleaned-up-iniParseLine-0001.bin -------------- next part -------------- A non-text attachment was scrubbed... Name: 0003-Some-minor-cleanup-in-iniLoadOptionsFromFile-and-iniGetFileDataFromFilename.patch Type: text/x-patch Size: 5291 bytes Desc: not available Url : http://lists.freedesktop.org/archives/compiz/attachments/20070413/ad78562a/0003-Some-minor-cleanup-in-iniLoadOptionsFromFile-and-iniGetFileDataFromFilename-0001.bin -------------- next part -------------- A non-text attachment was scrubbed... Name: 0004-Fixed-list-parsing.patch Type: text/x-patch Size: 702 bytes Desc: not available Url : http://lists.freedesktop.org/archives/compiz/attachments/20070413/ad78562a/0004-Fixed-list-parsing-0001.bin -------------- next part -------------- A non-text attachment was scrubbed... Name: 0005-Some-further-minor-fixes-to-ini.c.patch Type: text/x-patch Size: 5156 bytes Desc: not available Url : http://lists.freedesktop.org/archives/compiz/attachments/20070413/ad78562a/0005-Some-further-minor-fixes-to-ini.c-0001.bin
2007/4/13, Mike Dransfield <mike@blueroot.co.uk>:> Patrick Niklaus wrote: > > Hi, > > > > ok I hope this patches are now ok for you to commit. > > Thanks. > > The part which worries me is this > > - for (i=0; i<len; i++) > - { > - if (filename[i] == '-') > - { > - if (!pluginSep) > - pluginSep = i-1; > - else > - return NULL; /*found a second dash */ > - } > - else if (filename[i] == '.') > - { > - if (!screenSep) > - screenSep = i-1; > - else > - return NULL; /*found a second dot */ > - } > - } > + /* the split point for the plugin name */ > + pluginSep = strrchr(filename, '-'); > + if (!pluginSep) > + return NULL; > + > + /* the split point for the screen name */ > + screenSep = strrchr(filename, '.'); > + if (!screenSep) > + return NULL; > > It is in a patch marked minor cleanup but it is actually changing > the functionality slightly. > > In the original version I was keen to reject bad files as early as > possible so they couldn't cause any damage later. Thats why I was > checking the entire filename. > > Can you explain why you changed this bit (other than a slight speed > increase)? > > > >First point is, as you mentioned, a speed increase. Second point is that it takes less space and looks much cleaner. But if you are worried about files with names like "foo.bla-hehe" (which would be invalid) I could add a check for that of course. Regards, Patrick