[PATCH 4/4] gdb: Avoid signed integer overflow when printing source lines

Tom Tromey tom@tromey.com
Wed Jan 9 01:08:00 GMT 2019


>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The solution in this patch is a new class source_lines_range that can
Andrew> be constructed from a single line number and a direction (forward or
Andrew> backward).  The range is then constructed from the line number and the
Andrew> value of get_lines_to_list.

Sorry I didn't get to this the other night.

Andrew> +      source_lines_range range (sal_end.line + 1,
Andrew> +				source_lines_range::direction::backward);

Normally I think gdb uses upper case for enumerators.

Andrew> +  /* When constructing the range from a single line number, does the line
Andrew> +     range extend forward, or backward.  */
Andrew> +  enum class direction
Andrew> +  {
Andrew> +   forward,
Andrew> +   backward
Andrew> +  };

For an enum that's already nested in a class, I would probably just use
plain "enum" and not "enum class", since IMO the extra namespacing just
results in noise -- "source_lines_range::backward" seems just as clear,
but shorter, than "source_lines_range::direction::backward".

I don't insist on it though, I just thought I'd throw this out there to
see what others think.  As far as I know we don't have a rule about
this.

Andrew> +  /* Construct a SOURCE_LINES_RANGE starting at STARTLINE and extending in
Andrew> +   direction DIR.  The number of lines is from GET_LINES_TO_LIST.  If the
Andrew> +   direction is backward then the start is actually (STARTLINE -
Andrew> +   GET_LINES_TO_LIST).  There is also logic in place to ensure the start
Andrew> +   is always 1 or more, and the end will be at most INT_MAX.  */
Andrew> +  source_lines_range (int startline, direction dir = direction::forward);

I think this should be marked "explicit".  I guess it could go either
way but seeing as the patch uses it in an explicit way, and since
explicit is generally better...

Tom



More information about the Gdb-patches mailing list