[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Clean up a couple of compiler warnings

From: Philipp Stephani
Subject: Re: [PATCH] Clean up a couple of compiler warnings
Date: Fri, 19 May 2017 09:31:39 +0000

Paul Eggert <address@hidden> 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 work
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 == *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 == 2
code let's just #ifdef it out. I did that in the 4th attached patch. Maybe 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/ManPages/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/ManPages/man2/getattrlist.2.html), it just needs to be implemented correctly. I'd suggest to only use a working version of getattrlist on macOS.

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: 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öschen Sie die E-Mail und alle Anhänge. 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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]