From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Philipp Stephani Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Clean up a couple of compiler warnings Date: Fri, 19 May 2017 09:31:39 +0000 Message-ID: References: <20170518202450.75747-1-phst@google.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="94eb2c0b9ba2e66923054fdd30a4" X-Trace: blaine.gmane.org 1495186327 8784 195.159.176.226 (19 May 2017 09:32:07 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 19 May 2017 09:32:07 +0000 (UTC) To: Paul Eggert , Philipp Stephani , emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri May 19 11:32:01 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dBeGP-00027E-8D for ged-emacs-devel@m.gmane.org; Fri, 19 May 2017 11:32:01 +0200 Original-Received: from localhost ([::1]:57435 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dBeGU-0005ma-ES for ged-emacs-devel@m.gmane.org; Fri, 19 May 2017 05:32:06 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46740) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dBeGI-0005lE-Uy for emacs-devel@gnu.org; Fri, 19 May 2017 05:31:59 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dBeGF-0005Rr-J4 for emacs-devel@gnu.org; Fri, 19 May 2017 05:31:54 -0400 Original-Received: from mail-qk0-x22e.google.com ([2607:f8b0:400d:c09::22e]:36471) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dBeGF-0005RW-A3 for emacs-devel@gnu.org; Fri, 19 May 2017 05:31:51 -0400 Original-Received: by mail-qk0-x22e.google.com with SMTP id u75so56323119qka.3 for ; Fri, 19 May 2017 02:31:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=9qnfdMVEoO3gD+P/Y1DKkmcldrm1fvBhxVWK2d6Es3I=; b=rIx/bAb+1vSI6/wnxKk/YHf8JGPFIvwCGdS7inugrky+mxR3Hu1W0RWlOl93h/CijV IFSrxDgvz7E5ZOSreLZQ101MyLqqQXeTsMgfeoOKhavlfDJXwVFkc7ja4aTYzO0JKmvA Rsu6x/BBX0mUSnWmk4Jg5PA8RO7tVvedXBq9WVvzhHj427s2GdtkebpxmzAgTZg4ADT7 swBVPJ2gAOJpVvhnHP1fVR6IyEAXJIob+uBYjfX6NVkv9z9RKzxTWILT/NHiWTCQJmTr 15ADt0wGq4A+C9yykgM+W4w1wFkwKxeca9dFzcXaHC3bwKCdLMXo3NTBC6oX/nWqBpJa nluA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=9qnfdMVEoO3gD+P/Y1DKkmcldrm1fvBhxVWK2d6Es3I=; b=LkLM0Ri8x+klQan+gfM865Gy40UZe/pvTV5wUt1nw779Qn2gKQEFwxCDippchjNZ+k ArdKYUfap1G+Slq7QqqThVKo1Y1NisQEdn0Ae85HghzYs/ceGr3kXS5l+vGoc8crYUAg ToJoCveDiyY2Oin2HsqPHhURqD8xhkgq1z23dAydwBrT+HTX23J4jLPLnAmzCHrHRWwk bhzCbYMKcwEfQC1v7DhmC4IKRj3r5diPFs8bC2kc/Ewz2n2tw3eiCCx3opm/uIbIp5m0 suKkZPirfQ+kIOIJsI5YrXV93BVmfnrh15TgN+c/XGhZ0rFn6zpd/KXTiXN5Lf8+939V FcSA== X-Gm-Message-State: AODbwcBuC4KAHOmvg3wLak66eI/Ejh6q1mIAgfR/nVAJd3ZpUCjqREf9 zULaLLlEjOr0eYim30J/qFiBy0YRD4jl X-Received: by 10.55.8.2 with SMTP id 2mr7473939qki.269.1495186310291; Fri, 19 May 2017 02:31:50 -0700 (PDT) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:400d:c09::22e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:214986 Archived-At: --94eb2c0b9ba2e66923054fdd30a4 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Paul Eggert schrieb am Fr., 19. Mai 2017 um 05:48 Uhr: > > * emacs.c (using_utf8): Don't assume anything about mbstate_t type. > > The old code doesn't assume anything about the type either, and should wo= rk > regardless of how mbstate_t is implemented. The proposed change makes the > code a > bit harder to read, so I'd rather avoid it if possible. I couldn't > reproduce the > warning that you're evidently seeing. > Apparently on some systems mbstate_t is a nested struct, and the compiler warns about missing braces. Note that memset to initialize a mbstate_t is explicitly recommended in the libc manual: https://www.gnu.org/software/libc/manual/html_node/Keeping-the-state.html > > > > * emacs-module.c (MODULE_SETJMP_1): Mark dummy variable as unused. > > Rather than do that, let's just use the variable; that's more robust. I > installed the 2nd attached patch. > OK. If you want, you could now add eassert (handlerlist =3D=3D *dummy); or so to the cleanup function. > > > * lread.c (string_to_number): Use constants of double type. > > * editfns.c (decode_float_time): > > * fns.c (make_hash_table, maybe_resize_hash_table) > > (Fhash_table_rehash_size, Fhash_table_rehash_threshold): > > Explicitly cast floating-point values. > > There should be no problem (i.e., no loss of info) converting a float to = a > double, so these changes are not needed. I assume you're using clang. I > reproduced its false alarms on Fedora 25 x86-64, which uses clang 3.9.1, > and > installed the 3rd attached patch to work around the problem. > Looks good. I'm no fan of silencing implicit conversion warnings by introducing explicit casts either. > > > > * fileio.c (file_name_case_insensitive_p): Add cast. > > Rather than waste time static-checking the DARWIN_OS_CASE_SENSITIVE_FIXME > =3D=3D 2 > code let's just #ifdef it out. I did that in the 4th attached patch. Mayb= e > we > should just remove it, since nobody is using it and (as you note) it > doesn't > work anyway. > I think none of the four branches there work for macOS. https://developer.apple.com/legacy/library/documentation/Darwin/Reference/M= anPages/man2/pathconf.2.html is silent about case sensitivity, so we're already relying on some undocumented functionality. The getattrlist method is at least documented ( https://developer.apple.com/legacy/library/documentation/Darwin/Reference/M= anPages/man2/getattrlist.2.html), it just needs to be implemented correctly. I'd suggest to only use a working version of getattrlist on macOS. --=20 Google Germany GmbH Erika-Mann-Stra=C3=9Fe 33 80636 M=C3=BCnchen Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Gesch=C3=A4ftsf=C3=BChrer: Matthew Scott Sucherman, Paul Terence Manicle Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und l=C3=B6schen Sie die E-Mail und alle Anh=C3=A4nge. Vielen Dank. This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks. --94eb2c0b9ba2e66923054fdd30a4 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Paul E= ggert <eggert@cs.ucla.edu> = schrieb am Fr., 19. Mai 2017 um 05:48=C2=A0Uhr:
> * emacs.c (using_utf8): Don't assume anything about = mbstate_t type.

The old code doesn't assume anything about the type either, and should = work
regardless of how mbstate_t is implemented. The proposed change makes the c= ode a
bit harder to read, so I'd rather avoid it if possible. I couldn't = reproduce the
warning that you're evidently seeing.

<= div>Apparently on some systems mbstate_t is a nested struct, and the compil= er warns about missing braces. Note that memset to initialize a mbstate_t i= s explicitly recommended in the libc manual:=C2=A0https://www.gn= u.org/software/libc/manual/html_node/Keeping-the-state.html
= =C2=A0


> * emacs-module.c (MODULE_SETJMP_1): Mark dummy variable as unused.

Rather than do that, let's just use the variable; that's more robus= t. I
installed the 2nd attached patch.

OK. I= f you want, you could now add
eassert (handlerlist =3D=3D *dummy)= ;
or so to the cleanup function.
=C2=A0

> * lread.c (string_to_number): Use constants of double type.
> * editfns.c (decode_float_time):
> * fns.c (make_hash_table, maybe_resize_hash_table)
> (Fhash_table_rehash_size, Fhash_table_rehash_threshold):
> Explicitly cast floating-point values.

There should be no problem (i.e., no loss of info) converting a float to a<= br> double, so these changes are not needed. I assume you're using clang. I=
reproduced its false alarms on Fedora 25 x86-64, which uses clang 3.9.1, an= d
installed the 3rd attached patch to work around the problem.

Looks good. I'm no fan of silencing implicit conv= ersion warnings by introducing explicit casts either.
=C2=A0


=C2=A0> * fileio.c (file_name_case_insensitive_p): Add cast.

Rather than waste time static-checking the DARWIN_OS_CASE_SENSITIVE_FIXME = =3D=3D 2
code let's just #ifdef it out. I did that in the 4th attached patch. Ma= ybe we
should just remove it, since nobody is using it and (as you note) it doesn&= #39;t
work anyway.

I think none of the four b= ranches there work for macOS. https://developer.apple.com/legacy/library/documentation/Darwi= n/Reference/ManPages/man2/pathconf.2.html=C2=A0is silent about case sen= sitivity, so we're already relying on some undocumented functionality. = The getattrlist method is at least documented (https://developer.apple.com/legacy/library= /documentation/Darwin/Reference/ManPages/man2/getattrlist.2.html), it j= ust needs to be implemented correctly. I'd suggest to only use a workin= g version of getattrlist on macOS.
--

Google Germany GmbH
Erika-Mann-Stra=C3=9Fe 33
80636 M=C3=BCnchen

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Gesch=C3=A4ftsf=C3=BChrer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Ad= ressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absen= der und l=C3=B6schen Sie die E-Mail und alle Anh=C3=A4nge. Vielen Dank.

This e-mail is confidential. If you are not the right addres= see please do not forward it, please inform the sender, and please erase th= is e-mail including any attachments. Thanks.

--94eb2c0b9ba2e66923054fdd30a4--