speechd-discuss
[Top][All Lists]
Advanced

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

[PATCH] Fix most warnings with gcc 4.4 on Linux


From: Andrei Kholodnyi
Subject: [PATCH] Fix most warnings with gcc 4.4 on Linux
Date: Wed, 8 Sep 2010 13:07:20 +0200

>>> + ? ? ? DBG("Espeak: Requested data: |%s| %d %ld", data, msgtype, (long) 
>>> bytes);
>> why you cast here? just change to %d
>
> bytes is size_t. size_t can be larger than int (%d). I think there
> is no guarantee of long being larger than size_t either, but in
> practice will not happen.

sorry, I missed it is size_t. Then you need to cast, that's true.
Is it not %lu then?

>>> +static const struct option spd_long_options_[] = {
>>> + ? ?{"run-daemon", 0, 0, 'd'},
>>> + ? ?{"run-single", 0, 0, 's'},
>>> + ? ?{"spawn", 0, 0, 'a'},
>>> + ? ?{"log-level", 1, 0, 'l'},
>>> + ? ?{"communication-method", 1, 0, 'c'},
>>> + ? ?{"socket-path", 1, 0, 'S'},
>>> + ? ?{"port", 1, 0, 'p'},
>>> + ? ?{"pid-file", 1, 0, 'P'},
>>> + ? ?{"config-file", 1, 0, 'C'},
>>> + ? ?{"version", 0, 0, 'v'},
>>> + ? ?{"debug", 0, 0, 'D'},
>>> + ? ?{"help", 0, 0, 'h'},
>>> + ? ?{0, 0, 0, 0}
>>> +};
>>> +const struct option *const spd_long_options = spd_long_options_;
>> this i didn't get. why you are not using structure directly, but
>> introducing pointer to it instead?
>
> I could not declare in the header the variable without specific array size:
> struct option spd_long_options[]; // produces error
> so I had to declare it as pointer. And in the implementation file, declaring
> spd_long_options as pointer like this:
> ?... * spd_long_options = { ...
> lead to load of warnings.
> This was the only way to resolve this.
>
> This did not look ideal to me, but I am not aware of a better way.
>

you do not need to declare it in header.
if you have in options.c

static const struct option spd_long_options[] = {
     <your options>
};

c_opt = getopt_long(argc, argv, spd_short_options, spd_long_options,
                            &option_index);

it should produce no warnings.
why do you need an extra pointer? const struct option *const
spd_long_options = spd_long_options_;

>>> +const char *const spd_short_options = "dsal:c:S:p:P:C:vDh";
>> should it be static?
>
> no, since then it would not be visible outside of the options.c file.
> More precisely, either it can be made static and its declaration removed
> from options.h, or it can remain as it is in my patch.

yes, declaration removed from options.h, nobody needs it outside options.c

>
>>> +#define _GNU_SOURCE
>> this shall be defined via -D in Makefile
>
> saw these defined directly in source, so took my inspiration from
> that. But you are correct. (_GNU_SOURCE would ideally be not defined
> at all, but that's for future).

agree, it shouldn't be used,
but for now IMO it would be better to keep it in one place instead of
spreading across headers
and this place is Makefile



reply via email to

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