[Geany-devel] msgwin line and column position

Lex Trotman elextr at xxxxx
Thu Aug 18 02:28:01 UTC 2011


> Yes. After looking at the error_regex persing, default compiler parsing
> and default message parsing, I was not satisfied with any of them, and
> so will do something Lex-like: propose a global unified solution. After
> a few more days to think about it, that is.

Yes, better to get it right slowly than wrong quickly :-)

>
>> Comment on the patch:
>>
>> Documentation says:
>>
>> 17 +    whether the first or second match is purely digits.
>>
>> but (if I read the patch correctly) it actually only checks the first
>> and decides line [col] file or file line [col]
>
> "if (strtopos(match[1], line, TRUE) != NULL)" checks if the 2nd match is
> purely digits. line_idx is assigned before it for the match[line_idx]
> and possibly match[line_idx + 1] to be freed.

Ok.

>
>> In filetypes.c/filetypes_parse_error_message()
>>
>> 64 +          pmatch[2] = pmatch[3];
>>
>> I don't think its necessarily legal or safe to assign one match to
>> another, an opaque structure like regmatch_t might have pointers to
>> internal data and this might result in double free or cause other
>> problems.
>
> regex_t is opaque, while regmatch_t is documented in regexec(1) as
> having only rm_so and rm_eo. Even if there are more, a regmatch_t is
> never freed or anything.
>

Well you obviously get away with it because it isn't really opaque (to
the compiler), but it is still bad practice to assign such structures
unless it is documented safe.

>> The Geany convention is that symbols exported start with the prefix of
>> the file they are exported from, and that words be separated by
>> underlines, so strtopos should be utils_str_to_pos.  That also makes
>> it obvious that its a local function not a stdlib one.
>
> Hmm, yes. My bad.
>
>> In strtopos() it would be better to use strtoul(), no need to check l
>> >= 0 and the (int)l == l s/b l <= G_MAXINT since the return is a gint.
>
> Indeed... (gint) l == l must be checked though, unsigned long and gint
> have different ranges.

Technically (gint)l == l is using implementation defined behavior, the
standard says that the converting to a shorter signed type should give
the same value if it fits, but if it doesn't fit, its implementation
defined.  On the other hand since the range of long is greater than or
equal to the range in int which is what gint is, then the comparison
to G_MAXINT is a correct way of doing it.

>
>> > Now the two questions, mentioned in navqueue.c:
>> >
>> > How about supporting line number 0? process_build_output_line() does,
>> > and I, too, remember some old compilers emiting line 0.
>>
>> Sounds ok, line 0 == line 1?
>
> Sure. My *cc at work emits line 0 sometimes...

I suppose thats as good as any for errors it doesn't have a line
number for, so maybe we should use rand to pick a line number :-) oh,
ok line 1.

[...]
> On a 2nd thought, at this point we already switched to the file, so
> the filename was correct. Exiting navqueue_goto_line_col() with FALSE
> only prevents Enter and double click from focusing the editor, which is
> not very helpful. Addons/Tasks uses the return value, but doesn't seem
> like it really needs it. A void function will quickly solve the "what
> result to return if this or that argument is invalid or not entirely
> valid" question, but will have to wait until the release, so let's only
> add support for line = 0 for now.

Sounds fine.

Cheers
Lex



More information about the Devel mailing list