From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#43439: [PATCH] doprnt improvements Date: Wed, 16 Sep 2020 15:09:50 -0700 Organization: UCLA Computer Science Department Message-ID: <31fb34a7-26c3-552d-e8d4-74624455ffcb@cs.ucla.edu> References: <20200916015051.20517-1-eggert@cs.ucla.edu> <83o8m57oq7.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="5905"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 Cc: 43439@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Sep 17 00:11:07 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kIfdr-0001Qt-8R for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 17 Sep 2020 00:11:07 +0200 Original-Received: from localhost ([::1]:51856 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kIfdq-0006AM-9v for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 16 Sep 2020 18:11:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53184) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kIfco-0005Ru-BQ for bug-gnu-emacs@gnu.org; Wed, 16 Sep 2020 18:10:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:53409) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kIfco-0005xQ-1G for bug-gnu-emacs@gnu.org; Wed, 16 Sep 2020 18:10:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kIfcn-0005L4-Pe for bug-gnu-emacs@gnu.org; Wed, 16 Sep 2020 18:10:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 16 Sep 2020 22:10:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 43439 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 43439-submit@debbugs.gnu.org id=B43439.160029419920510 (code B ref 43439); Wed, 16 Sep 2020 22:10:01 +0000 Original-Received: (at 43439) by debbugs.gnu.org; 16 Sep 2020 22:09:59 +0000 Original-Received: from localhost ([127.0.0.1]:36722 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kIfcl-0005Kk-48 for submit@debbugs.gnu.org; Wed, 16 Sep 2020 18:09:59 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:53562) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kIfcj-0005KX-Jl for 43439@debbugs.gnu.org; Wed, 16 Sep 2020 18:09:58 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 2F46A16013B; Wed, 16 Sep 2020 15:09:52 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id kBG_5QO4ZFsU; Wed, 16 Sep 2020 15:09:51 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 344F316013C; Wed, 16 Sep 2020 15:09:51 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id C5FQTgDC4CYh; Wed, 16 Sep 2020 15:09:51 -0700 (PDT) Original-Received: from [192.168.1.9] (cpe-75-82-69-226.socal.res.rr.com [75.82.69.226]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 0C5FB16013B; Wed, 16 Sep 2020 15:09:51 -0700 (PDT) Autocrypt: addr=eggert@cs.ucla.edu; prefer-encrypt=mutual; keydata= LS0tLS1CRUdJTiBQR1AgUFVCTElDIEtFWSBCTE9DSy0tLS0tCgptUUlOQkV5QWNtUUJFQURB QXlIMnhvVHU3cHBHNUQzYThGTVpFb243NGRDdmM0K3ExWEEySjJ0QnkycHdhVHFmCmhweHhk R0E5Smo1MFVKM1BENGJTVUVnTjh0TFowc2FuNDdsNVhUQUZMaTI0NTZjaVNsNW04c0thSGxH ZHQ5WG0KQUF0bVhxZVpWSVlYL1VGUzk2ZkR6ZjR4aEVtbS95N0xiWUVQUWRVZHh1NDd4QTVL aFRZcDVibHRGM1dZRHoxWQpnZDdneDA3QXV3cDdpdzdlTnZub0RUQWxLQWw4S1lEWnpiRE5D UUdFYnBZM2VmWkl2UGRlSStGV1FONFcra2doCnkrUDZhdTZQcklJaFlyYWV1YTdYRGRiMkxT MWVuM1NzbUUzUWpxZlJxSS9BMnVlOEpNd3N2WGUvV0szOEV6czYKeDc0aVRhcUkzQUZINmls QWhEcXBNbmQvbXNTRVNORnQ3NkRpTzFaS1FNcjlhbVZQa25qZlBtSklTcWRoZ0IxRApsRWR3 MzRzUk9mNlY4bVp3MHhmcVQ2UEtFNDZMY0ZlZnpzMGtiZzRHT1JmOHZqRzJTZjF0azVlVThN Qml5Ti9iClowM2JLTmpOWU1wT0REUVF3dVA4NGtZTGtYMndCeHhNQWhCeHdiRFZadWR6eERa SjFDMlZYdWpDT0pWeHEya2wKakJNOUVUWXVVR3FkNzVBVzJMWHJMdzYrTXVJc0hGQVlBZ1Jy NytLY3dEZ0JBZndoU In-Reply-To: <83o8m57oq7.fsf@gnu.org> Content-Language: en-US X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:188182 Archived-At: On 9/16/20 7:58 AM, Eli Zaretskii wrote: > Emacs traditionally supports strings with > embedded null characters, and this feature is in line with that. It > is true that it is currently unused, but why is it a good idea to > remove it? It's a good idea not only because the feature is currently unused and its support complicates and adds bugs to the code, but also because it would be a bad idea to ever use the feature. The Emacs feature is for Lisp strings. Emacs does not (and for API reasons, it cannot practically) rely on embedded NULs in C strings. Among other things, if we tried to use C-style printf formats with embedded NULs, GCC's warnings about formats not matching their arguments would stop working. These GCC warnings are quite useful for preventing bugs in Emacs's C code and have helped to catch many such bugs, and we should not give them up. More generally, the vestigial support for NULs and %S in doprnt's C formats dates back to long ago, before GCC warned about these features. It may have made sense back then but it does not make sense now. Any C-level formatting facility that supports NULs and %S should not attempt to use the longstanding printf API that is incompatible with such support - it should be a separate facility. And Emacs C code already has a non-printf facility for formatting with NULs and %S - e.g., Fformat and Fformat_message - so it doesn't need yet another one. > If the problem is the slight inefficiency caused by the call to > strlen, we could instead solve it in the callers: all the formats I've > seen are const strings, so the value of FORMAT_END can be computed at > compile time, and used instead of passing NULL. This would require unnecessary complication of the code and runtime overhead. doprnt is called directly only in few places: esprintf, evxprintf, vmessage, and the (rarely-if-ever used) snprintf replacement. If we modified callers to do what you're suggesting, we'd need to modify the callers of these functions, and their callers too, all the way back to the original ancestor call that specifies the format. This would clutter and bloat the code and would add runtime cost to all the callers. For example, we'd have to change all calls to the 'error' function from something like this: if (ret < GNUTLS_E_SUCCESS) error ("GnuTLS AEAD cipher %s/%s initialization failed: %s", gnutls_cipher_get_name (gca), desc, emacs_gnutls_strerror (ret)); to something like this: if (ret < GNUTLS_E_SUCCESS) { char const msg[] = "GnuTLS AEAD cipher %s/%s initialization failed: %s"; error (msg, sizeof msg - 1, gnutls_cipher_get_name (gca), desc, emacs_gnutls_strerror (ret)); } Of course we could invent a new macro ERROR to package this up, but such a macro would still be less efficient than what we have, and worse it would not always work, for cases like this: if (NILP (tem) || (XBUFFER (tem) != current_buffer)) error (for_region ? "The mark is not set now, so there is no region" : "The mark is not set now"); or like this: error (format, string); // in x_check_errors And of course this could be gotten around as well, but we're now talking about a reasonably large amount of code surgery that will hurt code readability and runtime performance, all to support a low-level C feature that Emacs does not use and won't ever reasonably use. Instead, we should leave most of the C code alone and just adjust 'doprnt' and its very few callers. >> Drop support for >> "%S" which is never used and which would cause GCC to warn anyway. > > This is an old compatibility feature, I'd rather not drop it. Who > knows what code relies on the fact that 'message' and 'format-message' > support it? I know because I checked all the code. No Emacs C code uses %S. And none is likely to use it in the future because GCC warns about it if you try. (To be specific: GCC warns unless you use %S compatibly with its standard C meaning, which differs from that of Emacs doprnt - which is yet another compatibility minefield if we insist on keeping doprnt's unused %S feature.)