On Sat, Sep 03, 2011 at 07:12:40PM +0300, Henri Kemppainen wrote: > > Being a little less stupid this time. Rewrote the string handling > > part, style(9) formatting and refactored pushtag function. > > Some thoughts about this. Are you sure you're not going to overflow > t in re_tag_conv when you insert escapes? Correct me if I'm wrong, > but I think ctags patterns aren't really regexes (which is why you're > playing with escapes, and stripping & re-inserting magic) -- wouldn't > it be easier to do the matching in a plain loop, checking the start/end > offsets against bol/eol if the magic anchors were present?
The assumption in "ex" is that "t" is of double the size of "s" so that there is enough room to insert escapes in the worst case if every character needs to be escaped. Yes, "t" should be of size NLINE * 2 + 1. Plain loop matching would work provided ctags(1) doesn't escape every "/" with a "\". I have a earlier diff which does this simple char to char comparsion but it failed with these backslashes and that is when I looked at "ex" code and I felt it much cleaner than rolling my own. Oh and I almost forgot lines in mg aren't NUL terminated. > > Second, it seems you're allocating a number of strings in addctag. > This isn't technically wrong, but one of the upsides of NUL-terminated > strings is that you can split one into many (as strsep does) without > messing with multiple buffers & allocs & frees. > Yes, that is possible and never crossed my mind. Will try to come up with a new diff. > I'd supplement ctagnode with a flag field to tell which magic anchors > were present originally. I'd actually strip all the magic and backslashes > in addctag, such that pattern pointed to by pat is simply a plain string > which must exactly match what's in the buffer, character by character -- > making the match loop I proposed above a trivial one. > > Finally, the choice to use underscores in the name of one function stands > out as a little bit odd ;-) > I am trying to be consistent with names here, all init methods were indeed named xxxx_init. > That's what I got at a quick glance. I'll try give it another look once > this fever and headache fade. > get well soon :)
