From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19449 invoked by alias); 4 Nov 2013 11:58:01 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 19438 invoked by uid 89); 4 Nov 2013 11:58:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_HELO_PASS,SPF_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Nov 2013 11:57:58 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rA4Bvpfw015222 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 4 Nov 2013 06:57:51 -0500 Received: from tucnak.zalov.cz (vpn1-5-212.ams2.redhat.com [10.36.5.212]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rA4Bvn8x030925 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 4 Nov 2013 06:57:50 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.7/8.14.7) with ESMTP id rA4Bvm9l022591; Mon, 4 Nov 2013 12:57:48 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.7/8.14.7/Submit) id rA4BvmaR022590; Mon, 4 Nov 2013 12:57:48 +0100 Date: Mon, 04 Nov 2013 11:59:00 -0000 From: Jakub Jelinek To: Dodji Seketeli Cc: Bernd Edlinger , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH] preprocessor/58580 - preprocessor goes OOM with warning for zero literals Message-ID: <20131104115748.GO27813@tucnak.zalov.cz> Reply-To: Jakub Jelinek References: <20131031144309.GR27813@tucnak.zalov.cz> <87y559xz7y.fsf@redhat.com> <20131031173649.GW27813@tucnak.zalov.cz> <87r4awtmnx.fsf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87r4awtmnx.fsf@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg00199.txt.bz2 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 than MAX_WIDTH minus the margin. The > margin is either 10 characters or the difference between the column > - and the length of the line, whatever is smaller. */ > + and the length of the line, whatever is smaller. The length of > + LINE is given by LINE_WIDTH. */ > static const char * > -adjust_line (const char *line, int max_width, int *column_p) > +adjust_line (const char *line, int line_width, > + int max_width, int *column_p) 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 should not crash or overflow buffers on it ;). Anyway, not something needed to be fixed right now, but in the future it would be nicer to use size_t and/or ssize_t here. > { > int right_margin = 10; > - int line_width = strlen (line); > int column = *column_p; > > right_margin = MIN (line_width - column, right_margin); > @@ -284,6 +285,7 @@ diagnostic_show_locus (diagnostic_context * context, > const diagnostic_info *diagnostic) > { > const char *line; > + int line_width; > char *buffer; > expanded_location s; > int max_width; > @@ -297,22 +299,25 @@ diagnostic_show_locus (diagnostic_context * context, > > context->last_location = diagnostic->location; > s = expand_location_to_spelling_point (diagnostic->location); > - line = location_get_source_line (s); > + line = location_get_source_line (s, line_width); I think richi didn't like C++ reference arguments to be used that way (and perhaps guidelines don't either), because it isn't immediately obvious that line_width is modified by the call. Can you change it to a pointer argument instead and pass &line_width? > + *lineptr = XNEWVEC (char, *n); > + if (*lineptr == NULL) > + return -1; XNEWVEC or XRESIZEVEC will never return NULL though, so it doesn't have to be tested. Though, the question is if that is what we want, caret diagnostics should be optional, if we can't print it, we just won't. So perhaps using malloc/realloc here would be better? > > const char * > -location_get_source_line (expanded_location xloc) > +location_get_source_line (expanded_location xloc, > + int& line_len) Ditto. Otherwise, LGTM. Jakub