Hi Tim, On Tue, Apr 21, 2009 at 03:48:49PM +0000, Tim Nieradzik wrote:
The patch has a strange comment WRT equal signs:
+ // parse the value + i = 0; + while (*p) { + // skip all spaces and equal signs + if (!isspace(*p)) { + value[i++] = *p; + } + + p++; + } + + value[i] = '\0';
So either the implementation or the comment is wrong...
The comment is wrong, I am sorry. If you look a few lines above, you will see that we have already checked whether the current character is an equal sign.
In a previous implementation I used to have an additional check there but removed it later as we do not want to skip any equal signs within values. Equal signs are very common in search URLs so skipping them could have led to many problems.
I guess I just forgot to adjust the comment accordingly after having changed this part.
If the required job is basically the same, I can keep the current function (maybe returning the new termination codes) and continue integrating the patchset.
Yes, that is probably best.
OK, a new patch for this was just committed. Several changes/fixes were made. Of the top of my head: - Remove multiple return points inside functions - Comments accuracy - Keep source code file margin < 80 cols. - Identation style in dillo uses 3 spaces. (and there was wrong indentation too). - FIxed a memory leak. Tim: please make a diff between a tree with the original patch and what was committed to get a detailed view of the changes. I'll continue integrating from here. -- Cheers Jorge.-