[PATCH] Fix most warnings with gcc 4.4 on Linux

[Boris Dusek]

Thanks for your review, Andrei, comments below:

On Sep 8, 2010, at 11:23 AM, Andrei Kholodnyi wrote:

>> +           if ((revents = id->alsa_poll_fds[count-1].revents)){
> revents = id->alsa_poll_fds[count-1].revents;
> if(0 != revent)

that is better

>> +    const bool action_is_open = strcmp (action, "--open") == 0;
> why you are using bool here? it is not a C standard. would it be
> possible to use int? or gboolean

bool is typedef'ed in the corresponding header (spdsend.h:42).
Int is OK as well. Int is actually better since bool from spdsend.h
is a bit opaque (uses TRUE, FALSE enum).

>> +/*
>>  static void cicero_set_pitch(signed int pitch);
>>  static void cicero_set_volume(signed int pitch);
>>  static void cicero_set_voice(EVoiceType voice);
>> +*/
> if these are not used, just remove them

correct, same for their empty definitions below

>> +       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.

>> +    DBG("Cache: cleaning, cache size %ld kbytes (>max %d).", (long) 
>> (FestivalCache.size/1024),
> same

same as the previous one. FestivalCache.size is size_t.

> 
>> +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.

>> +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.

>> +#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).

>> +               char buf[1];
> char buf;

correct :-)

Please comment on the points I disagreed with. I will post a revised
patch in the evening.

Boris