On Sun, 2016-10-02 at 13:07 +0000, Bernd Edlinger wrote:
> Hi Dave,
>
> here is the new version of the input.c patch:
>
> I have updated the comments, and revised the test case as requested.
> I have additionally done a bootstrap with build config=bootstrap
> -asan.
Thanks. A couple of nits inline below...
> Bootstrap and reg-testing on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 2016-09-26 Bernd Edlinger <bernd.edlin...@hotmail.de>
>
> PR preprocessor/77699
> * input.c (maybe_grow): Don't allocate one byte extra
headroom.
> (get_next_line): Return false on error.
> (read_next_line): Removed, use get_next_line instead.
> (read_line_num): Don't copy the line.
> (location_get_source_line): Don't use static data.
> (test_reading_source_line): Add more test cases.
FWIW I've been adding selftest:: to those symbols within the namespace
in ChangeLog entries, so I would have written this last one as:
(selftest::test_reading_source_line): Add more test cases.
(mostly out of wanting to emphasize the "real" code vs test code
split).
That said, I don't think we have any official policy on this.
> Index: gcc/input.c
> ===================================================================
> --- gcc/input.c (revision 240693)
> +++ gcc/input.c (working copy)
[...snip...]
> @@ -643,15 +612,15 @@ goto_next_line (fcache *cache)
> }
>
> /* Read an arbitrary line number LINE_NUM from the file cached in C.
> - The line is copied into *LINE. *LINE_LEN must have been set to
the
> - length of *LINE. If *LINE is too small (or NULL) it's extended
(or
> - allocated) and *LINE_LEN is adjusted accordingly. *LINE ends up
> - with a terminal zero byte and can contain additional zero bytes.
> + If the line was read successfully, *LINE points to the beginning
> + of the line in the file cache and *LINE_LEN is the length of the
> + line. *LINE is not nul-terminated, but may contain zero bytes.
> + *LINE is only valid until the next call of read_line_num.
> This function returns bool if a line was read. */
>
> static bool
> read_line_num (fcache *c, size_t line_num,
> - char ** line, ssize_t *line_len)
> + char **line, ssize_t *line_len)
> {
> gcc_assert (line_num > 0);
>
> @@ -705,12 +674,8 @@ read_line_num (fcache *c, size_t line_num,
> {
> /* We have the start/end of the line. Let's just copy
> it again and we are done. */
The reference to a "copy" in this comment is now invalid. Maybe the
comment should now simply read:
/* We have the start/end of the line. */
or somesuch.
> - ssize_t len = i->end_pos - i->start_pos + 1;
> - if (*line_len < len)
> - *line = XRESIZEVEC (char, *line, len);
> - memmove (*line, c->data + i->start_pos, len);
> - (*line)[len - 1] = '\0';
> - *line_len = --len;
> + *line = c->data + i->start_pos;
> + *line_len = i->end_pos - i->start_pos;
> return true;
> }
>
[...snip...]
OK for trunk with the above comment nit fixed.
Dave
