From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 8C7273858004 for ; Thu, 20 May 2021 23:04:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8C7273858004 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=segher@kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 14KN33wO010884; Thu, 20 May 2021 18:03:03 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 14KN339D010883; Thu, 20 May 2021 18:03:03 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Thu, 20 May 2021 18:03:02 -0500 From: Segher Boessenkool To: Bill Schmidt Cc: gcc-patches@gcc.gnu.org, jakub@redhat.com, jlaw@tachyum.com, dje.gcc@gmail.com Subject: Re: [PATCH 05/57] rs6000: Add file support and functions for diagnostic support Message-ID: <20210520230302.GZ10366@gate.crashing.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, TXREP, T_SPF_HELO_PERMERROR, T_SPF_PERMERROR autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 May 2021 23:04:05 -0000 Hi! On Tue, Apr 27, 2021 at 10:32:40AM -0500, Bill Schmidt via Gcc-patches wrote: > * config/rs6000/rs6000-gen-builtins.c (bif_file): New filescope > variable. What makes it interesting that this var has file scope? Did you mean to say it has internal linkage ("is static")? I would just leave that off completely, anyway (just "New." or "New variable.") > (LINELEN): New defined constant. It isn't a constant, it's a macro. "New macro."? > +#define LINELEN 1024 > +static char linebuf[LINELEN]; You never get anywhere close to 1024 I suppose? > +/* Pointer to a diagnostic function. */ > +void (*diag) (const char *, ...) __attribute__ ((format (printf, 1, 2))) > + = NULL; This isn't portable: you cannot assign NULL to a pointer to function. But it will work on all POSIX machines, and those are the only hosts we support. Still icky :-) If you just leave off the initialisation, it will be initialised to 0, which is exactly the same problem of course, just less explicit. This pointer is not static btw? Should it be? Okay for trunk with a little changelog massaging. Thanks! Segher