Re: [Ctags-devel] WindRes parser

[Frank F. <ff...@us...>]

2013/8/24 Colomban Wendling:
> First, the parser looks quite good and simple, apart from a few things
> I'll elaborate below, good job.

Thanks for the nice words.

> However, I have to warn you:  the ctags developers here aren't really
> responsive (I don't blame them, they probably have other things to do,
> no bashing), which means you may have to wait a looong time before you
> get an answer.  Either be very patient, or don't put too much hope on it.

I read your other mail and have seen the activity on this list is not
that high. Maybe our conversation will help ;-)

> Now, a few comments on the parser:
>
> * Watch string ends.
>   you do:
>
>         while (!isspace((int) *line)) ..
>
>   This is dangerous if the line only contains non-space characters,
> because you'll pass by the end of the string.  Just add a check for \0
> (string terminator):
>
>         while (*line && !isspace((int) *line))
>
>   or alike.

You're right. I've fixed that.

> * You may want to use vStringValue() instead of manually accessing
> str->buffer.  I guess it won't change much, but maybe if sometimes the
> vString implementation is changed it might help -- but that certainly is
> not a big deal.

Wasn't aware of this macro. Code looks better now.

> * Don't index strings at a negative offset:
>
>         else if( *line == '*' && *(line-1) == '/' )
>
>   might index the string at a negative index if '*' appears to be the
> first character on a line.  I would rather do it the other way around:
>
>         else if (*line == '/' && line[1] == '*')
>
>   since indexing at +1 is OK since it would only hit the string
> termination character (\0).

That's happens when you copy code from other parsers :-S
Fixed that as well.

I will send a new version soon. I first have to fix nested begin/end blocks.

>> Other than that I am also busy fixing the css parser that is available
>> at several locations on the net and is not yet included in the
>> official ctags. But I will send a separate mail about that parser when
>> I have fixed the problems (mainly related to css3 tags). This to keep
>> the conversation about the two parser separated from each other.
>
> Yep, good to separate mails so this don't get bloated by off-topic stuff.

I am working on a patch / suggestion to overcome the lack of
fnmatch(3) in the Windows c-libs. Expect another mail about that as
well.

Regards,
Frank