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

[Philipp Stephani <phst@google.com>]

[-- Attachment #1: Type: text/plain, Size: 3192 bytes --]

Paul Eggert <eggert@cs.ucla.edu> 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.

[-- Attachment #2: Type: text/html, Size: 4539 bytes --]