"H.J. Lu" writes:
> The new tests failed on Linux/x86:
Woops.
I have committed the patch below under the obvious rule for this. Sorry
for the inconvenience.
gcc/testsuite/ChangeLog:
* c-c++-common/cpp/warning-zero-location-2.c: Fix error message
specifier.
diff --git a/gcc/
On Tue, Jan 28, 2014 at 5:23 AM, Dodji Seketeli wrote:
> Dodji Seketeli writes:
>
>> Here is the patch I am committing right now.
>>
>> gcc/ChangeLog
>>
>> * input.c (location_get_source_line): Bail out on when line number
>> is zero, and test the return value of
>> lookup_or_ad
Dodji Seketeli writes:
> Here is the patch I am committing right now.
>
> gcc/ChangeLog
>
> * input.c (location_get_source_line): Bail out on when line number
> is zero, and test the return value of
> lookup_or_add_file_to_cache_tab.
>
> gcc/testsuite/ChangeLog
>
> * c-c++
Here is the patch I am committing right now.
gcc/ChangeLog
* input.c (location_get_source_line): Bail out on when line number
is zero, and test the return value of
lookup_or_add_file_to_cache_tab.
gcc/testsuite/ChangeLog
* c-c++-common/cpp/warning-zero-location.c
On 2014.01.25 at 00:02 +0100, Markus Trippelsdorf wrote:
> On 2014.01.24 at 17:09 +0100, Dodji Seketeli wrote:
> > Jakub Jelinek writes:
> >
> > > On Fri, Jan 24, 2014 at 04:40:52PM +0100, Dodji Seketeli wrote:
> > >> > The patch causes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59935 .
> > >> >
On 2014.01.24 at 17:09 +0100, Dodji Seketeli wrote:
> Jakub Jelinek writes:
>
> > On Fri, Jan 24, 2014 at 04:40:52PM +0100, Dodji Seketeli wrote:
> >> > The patch causes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59935 .
> >> > The follow-up patch (fp == NULL check) doesn't help.
> >>
> >> I am
On Fri, Jan 24, 2014 at 05:09:29PM +0100, Dodji Seketeli wrote:
> * input.c (read_line_num): Gracefully handle non-file locations or
> empty caches.
>
> diff --git a/gcc/input.c b/gcc/input.c
> index 547c177..b05e1da 100644
> --- a/gcc/input.c
> +++ b/gcc/input.c
> @@ -600,7 +600,8 @@
Jakub Jelinek writes:
> On Fri, Jan 24, 2014 at 04:40:52PM +0100, Dodji Seketeli wrote:
>> > The patch causes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59935 .
>> > The follow-up patch (fp == NULL check) doesn't help.
>>
>> I am looking into that, sorry for the inconvenience.
>
> I'd say we wa
On Fri, Jan 24, 2014 at 04:40:52PM +0100, Dodji Seketeli wrote:
> > The patch causes http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59935 .
> > The follow-up patch (fp == NULL check) doesn't help.
>
> I am looking into that, sorry for the inconvenience.
I'd say we want something like following. Not
Markus Trippelsdorf writes:
> On 2014.01.22 at 09:16 +0100, Dodji Seketeli wrote:
>> Bernd Edlinger writes:
>>
>> > Hi,
>>
>> Hello,
>>
>> > since there was no progress in the last 2 months on that matter,
>>
>> Sorry, this is my bad. I got sidetracked by something else and forgot
>> that I
On 2014.01.22 at 09:16 +0100, Dodji Seketeli wrote:
> Bernd Edlinger writes:
>
> > Hi,
>
> Hello,
>
> > since there was no progress in the last 2 months on that matter,
>
> Sorry, this is my bad. I got sidetracked by something else and forgot
> that I had the patch working et al, and all its
Jakub Jelinek writes:
> On Wed, Jan 22, 2014 at 09:16:02AM +0100, Dodji Seketeli wrote:
>> +static fcache*
>> +add_file_to_cache_tab (const char *file_path)
>> +{
>> +
>> + FILE *fp = fopen (file_path, "r");
>> + if (ferror (fp))
>> +{
>> + fclose (fp);
>> + return NULL;
>> +}
Hi,
On Thu, 23 Jan 2014 18:12:45, Jakub Jelinek wrote:
>
> On Wed, Jan 22, 2014 at 09:16:02AM +0100, Dodji Seketeli wrote:
>> +static fcache*
>> +add_file_to_cache_tab (const char *file_path)
>> +{
>> +
>> + FILE *fp = fopen (file_path, "r");
>> + if (ferror (fp))
>> + {
>> + fclose (fp);
>> + ret
On Wed, Jan 22, 2014 at 09:16:02AM +0100, Dodji Seketeli wrote:
> +static fcache*
> +add_file_to_cache_tab (const char *file_path)
> +{
> +
> + FILE *fp = fopen (file_path, "r");
> + if (ferror (fp))
> +{
> + fclose (fp);
> + return NULL;
> +}
I've seen various segfaults here w
Bernd Edlinger writes:
> Hi,
Hello,
> since there was no progress in the last 2 months on that matter,
Sorry, this is my bad. I got sidetracked by something else and forgot
that I had the patch working et al, and all its bits that need approval
got approved. It still can go in right now. It
Hi,
since there was no progress in the last 2 months on that matter,
and we are quite late in Phase 3 now,
I dare to propose an alternative, very simple solution for this bug now.
It does not try to improve or degade the perfomance at all, instead it simply
detects binary files with embedded NULs
> "Dodji" == Dodji Seketeli writes:
Dodji> * include/line-map.h (linemap_get_file_highest_location): Declare
Dodji> new function.
Dodji> * line-map.c (linemap_get_file_highest_location): Define it.
I wasn't sure if this is the patch you were needing review for ...
Dodji> +bool linemap_ge
Jakub Jelinek writes:
> On Tue, Nov 12, 2013 at 04:33:41PM +0100, Dodji Seketeli wrote:
>> +
>> + memmove (*line, l, len);
>> + (*line)[len - 1] = '\0';
>> + *line_len = --len;
>
> Shouldn't this be testing that len > 0 && (*line)[len - 1] == '\n'
> first before you decide to overw
On Tue, Nov 12, 2013 at 04:33:41PM +0100, Dodji Seketeli wrote:
> +
> + memmove (*line, l, len);
> + (*line)[len - 1] = '\0';
> + *line_len = --len;
Shouldn't this be testing that len > 0 && (*line)[len - 1] == '\n'
first before you decide to overwrite it and decrement len?
Though i
Sorry, I missed one question in the previous email.
Bernd Edlinger writes:
> and what is it if there is no terminal '\n' ?
In that case it's that the entire file is made of one line. For that
case get_next_line has allocated enough space for one
byte-passed-the-end of the file, so that there i
Bernd Edlinger writes:
>>> Using -- on a value that goes out of scope looks
>>> awkward IMHO.
>>
>> I don't understand this sentence. What do you mean by "Using -- on a
>> value that goes out of scope"?
>>
>
> I meant the operator -- in *line_len = --len;
Sorry, I don't see how that is an issu
>
>>> + memmove (*line, l, len);
>>> + (*line)[len - 1] = '\0';
>>> + *line_len = --len;
>>
>> Generally, I would prefer to use memcpy,
>> if it is clear that the memory does not overlap.
>
> I don't mind. I'll change that in my local copy. Thanks.
>
>> You copy one char too much and set it to zero
Bernd Edlinger writes:
>> + memmove (*line, l, len);
>> + (*line)[len - 1] = '\0';
>> + *line_len = --len;
>
> Generally, I would prefer to use memcpy,
> if it is clear that the memory does not overlap.
I don't mind. I'll change that in my local copy. Thanks.
> You copy one char too much and
Hi,
On Tue, 12 Nov 2013 16:33:41, Dodji Seketeli wrote:
>
> +/* Reads the next line from FILE into *LINE. If *LINE is too small
> + (or NULL) it is allocated (or extended) to have enough space to
> + containe the line. *LINE_LENGTH must contain the size of the
> + initial*LINE buffer. It's then up
Hello,
Below is the updated patch amended to take your previous comments in
account.
In add_file_to_cache_tab the evicted cache array entry is the one that
was less used.
Incidentally I also fixed some thinkos and issued that I have seen in
the previous patch.
Bootstrapped on x86_64-unknown-lin
Jakub Jelinek writes:
>> -OBJS-libcommon = diagnostic.o diagnostic-color.o pretty-print.o intl.o
>> input.o version.o
>> +OBJS-libcommon = diagnostic.o diagnostic-color.o pretty-print.o intl.o
>> vec.o input.o version.o
>
> Too long line?
Fixed in my local copy of the patch, thanks.
>
>> +
On Mon, Nov 11, 2013 at 11:19:21AM +0100, Dodji Seketeli wrote:
> .../c-c++-common/cpp/warning-zero-in-literals-1.c | Bin 0 -> 240 bytes
> libcpp/include/line-map.h | 8 +
> libcpp/line-map.c | 40 ++
> 8 files changed, 585 insertions(
Hello,
As it appeared that concerns about the speed of
location_get_source_line were as present as the need of just fixing
this bug, I have conflated the two concerns in a new attempt below,
trying to address the points you guys have raised during the previous
reviews.
The patch below introduces
Sorry Dodji,
I still do not see how this is supposed to work:
If the previous invocation of get_line already had read some
characters of the following line(s), how is that information
recovered?
I see it is copied behind lineptr[cur_len].
But when get_line is re-entered, cur_len is set to zero a
Bernd Edlinger writes:
> If you want to have at least a chance to survive something like:
>
>
> dd if=/dev/zero of=test.c bs=10240 count=1000
>
> gcc -Wall test.c
>
>
> Then you should change the implementation of read_line to
> _not_ returning something like 100GB of zeros.
I'd say that in
Hi,
you're welcome.
Just one more thought on the design.
If you want to have at least a chance to survive something like:
dd if=/dev/zero of=test.c bs=10240 count=1000
gcc -Wall test.c
Then you should change the implementation of read_line to
_not_ returning something like 100GB of zeros
Bernd Edlinger writes:
[...]
>> if (!string_len)
>> {
>> string_len = 200;
>> - string = XNEWVEC (char, string_len);
>> + string = XCNEWVEC (char, string_len);
>> }
>> + else
>> + memset (string, 0, string_len);
>
> Is this memset still necessary?
Of course not ...
[...]
> If "ptr" is passed
Hi,
On Mon, 4 Nov 2013 16:40:38, Dodji Seketeli wrote:
> +static ssize_t
> +get_line (char **lineptr, size_t *n, FILE *fp)
> +{
> + ssize_t cur_len = 0, len;
> + char buf[16384];
> +
> + if (lineptr == NULL || n == NULL)
> + return -1;
> +
> + if (*lineptr == NULL || *n == 0)
> + {
> + *n = 120;
>
Jakub Jelinek writes:
[...]
> Eventually, I think using int for sizes is just a ticking bomb, what if
> somebody uses > 2GB long lines? Surely, on 32-bit hosts we are unlikely to
> handle it, but why couldn't 64-bit host handle it? Column info maybe bogus
> in there, sure, but at least we shou
Bernd Edlinger writes:
> I see another "read_line" at gcov.c, which seems to be a copy.
>
> Maybe this should be changed too?
I have seen it as well.
I'd rather have the patch be reviewed and everthing, and only then
propose to share the implementation with the gcov module.
--
>
> On Mon, Nov 04, 2013 at 12:59:49PM +0100, Bernd Edlinger wrote:
>> I see another "read_line" at gcov.c, which seems to be a copy.
>
> Copy of what? gcov.c read_line hardly can be allowed to fail because out of
> mem unlike this one for caret diagnostics.
> Though, surely, this one could be some
On Mon, Nov 04, 2013 at 12:59:49PM +0100, Bernd Edlinger wrote:
> I see another "read_line" at gcov.c, which seems to be a copy.
Copy of what? gcov.c read_line hardly can be allowed to fail because out of
mem unlike this one for caret diagnostics.
Though, surely, this one could be somewhat adjust
Hi,
I see another "read_line" at gcov.c, which seems to be a copy.
Maybe this should be changed too?
What do you think?
Bernd.
On Mon, 4 Nov 2013 12:46:10, Dodji Seketeli wrote:
>
> Jakub Jelinek writes:
>
>> I think even as a fallback is the patch too expensive.
>> I'd say best would be to
On Mon, Nov 04, 2013 at 12:46:10PM +0100, Dodji Seketeli wrote:
> --- a/gcc/diagnostic.c
> +++ b/gcc/diagnostic.c
> @@ -259,12 +259,13 @@ diagnostic_build_prefix (diagnostic_context *context,
> MAX_WIDTH by some margin, then adjust the start of the line such
> that the COLUMN is smaller tha
Jakub Jelinek writes:
> I think even as a fallback is the patch too expensive.
> I'd say best would be to write some getline API compatible function
> and just use it, using fread on say fixed size buffer.
OK, thanks for the insight. I have just used the getline_fallback
function you proposed,
On Thu, Oct 31, 2013 at 04:00:01PM +0100, Dodji Seketeli wrote:
> Jakub Jelinek writes:
>
> > On Thu, Oct 31, 2013 at 03:36:07PM +0100, Bernd Edlinger wrote:
> >> if you want to read zero-chars, why don't you simply use fgetc,
> >> optionally replacing '\0' with ' ' in read_line?
> >
> > Because
On 31 October 2013 05:46, Dodji Seketeli wrote:
> +*/
> +static size_t
> +string_length (const char* buf, size_t buf_size)
> +{
> + for (int i = buf_size - 1; i > 0; --i)
> +{
> + if (buf[i] != 0)
> + return i + 1;
> +
> + if (buf[i - 1] != 0)
> + return i;
> +}
> +
Jakub Jelinek writes:
> On Thu, Oct 31, 2013 at 03:36:07PM +0100, Bernd Edlinger wrote:
>> if you want to read zero-chars, why don't you simply use fgetc,
>> optionally replacing '\0' with ' ' in read_line?
>
> Because it is too slow?
>
> getline(3) would be much better for this purpose, though o
On Thu, Oct 31, 2013 at 03:36:07PM +0100, Bernd Edlinger wrote:
> if you want to read zero-chars, why don't you simply use fgetc,
> optionally replacing '\0' with ' ' in read_line?
Because it is too slow?
getline(3) would be much better for this purpose, though of course
it is a GNU extension in
Hi,
if you want to read zero-chars, why don't you simply use fgetc,
optionally replacing '\0' with ' ' in read_line?
Bernd.
Hello,
In this problem report, the compiler is fed a (bogus) translation unit
in which some literals contains bytes whose value is zero. The
preprocessor detects that and proceeds to emit diagnostics for that
king of bogus literals. But then when the diagnostics machinery
re-reads the input file
46 matches
Mail list logo
