Re: [vdr] Logs from building VDR 2.1.3 with Clang 3.4.1

2014-02-09 Thread Paul Menzel
Dear Klaus,


thanks a lot for your quick reaction and for addressing the issues!


Am Samstag, den 08.02.2014, 13:51 +0100 schrieb Klaus Schmidinger:
> On 08.02.2014 09:59, Paul Menzel wrote:

> > building VDR 2.1.3 with Clang 3.4.1 the warnings below are shown.
> >
> > Most warnings look like they can be ignored. Maybe you can spot
> > something, which should be fixed.
> >
> >  $ clang --version
> >  Debian clang version 3.4-1 (tags/RELEASE_34/final) (based on LLVM 
> > 3.4)
> >  Target: i386-pc-linux-gnu
> >  Thread model: posix
> >  $ CC=clang CXX=clang++ make
> >  […]
> >  clang: warning: treating 'c' input as 'c++' when in C++ mode, this 
> > behavior is deprecated
> 
> You may want to find an option that suppresses this warning, because the file 
> names of VDR (*.c)
> are not going to change.

I know your point of view on that one from the same issue with Cppcheck,
so I did not comment that warning. ;-)

Looking at the manual of Clang, the option below looks promising.

   -ObjC++
   Treat source input files as Objective-C++ inputs.

[…]


Thanks,

Paul


PS: Also big thanks to Tony for helping addressing the warnings!


signature.asc
Description: This is a digitally signed message part
___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


Re: [vdr] Logs from building VDR 2.1.3 with Clang 3.4.1

2014-02-08 Thread Klaus Schmidinger

On 08.02.2014 16:10, Tony Houghton wrote:

On Sat, 08 Feb 2014 15:17:09 +0100
Klaus Schmidinger  wrote:


On 08.02.2014 14:34, Tony Houghton wrote:


The warning is justified, because if rid is 0 it's still there as an
argument, but just happens to have a value of 0. I think you can make
snprintf "consume" it without printing anything by adding %.d to the
second format string.


I'm afraid not.
If I run

#include 
int main(void)
{
for (int n = 0; n < 2; n++)
printf(n ? "'%d-%d'\n" : "'%d%.d'\n", 1, 2);
return 0;
}

I get

'12'
'1-2'

But maybe there *is* such a format character, it just isn't "%.d".


You're right, it looks like it has to be %.s, it doesn't work with %.d.
If you used %.s you'd probably just get a type mismatch warning instead
:-(. There's nothing wrong with the way you wrote it, but I like to
enable all possible warnings and eliminate them. In this case I think
I'd just print the rid even if it's 0.


The thing is that I'd rather get rid of the RID altogether at some point,
so I wouldn't want to manifest it by printing it if it is 0 ;-).


Can you suggest a different way of causing a segfault at this point?


You could add volatile as the warning suggests. Is there a good reason
not to use abort() instead?


The idea behind this was to allow for easy backtracking in such a case.
I believe abort() wouldn't allow this, would it?


abort() does usually generate a useful core dump/stack backtrace IME.


You're right, I've changed it.

Klaus

___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


Re: [vdr] Logs from building VDR 2.1.3 with Clang 3.4.1

2014-02-08 Thread Tony Houghton
On Sat, 08 Feb 2014 15:17:09 +0100
Klaus Schmidinger  wrote:

> On 08.02.2014 14:34, Tony Houghton wrote:
> >
> > The warning is justified, because if rid is 0 it's still there as an
> > argument, but just happens to have a value of 0. I think you can make
> > snprintf "consume" it without printing anything by adding %.d to the
> > second format string.
> 
> I'm afraid not.
> If I run
> 
> #include 
> int main(void)
> {
>for (int n = 0; n < 2; n++)
>printf(n ? "'%d-%d'\n" : "'%d%.d'\n", 1, 2);
>return 0;
> }
> 
> I get
> 
> '12'
> '1-2'
> 
> But maybe there *is* such a format character, it just isn't "%.d".

You're right, it looks like it has to be %.s, it doesn't work with %.d.
If you used %.s you'd probably just get a type mismatch warning instead
:-(. There's nothing wrong with the way you wrote it, but I like to
enable all possible warnings and eliminate them. In this case I think
I'd just print the rid even if it's 0.

> >> Can you suggest a different way of causing a segfault at this point?
> >
> > You could add volatile as the warning suggests. Is there a good reason
> > not to use abort() instead?
> 
> The idea behind this was to allow for easy backtracking in such a case.
> I believe abort() wouldn't allow this, would it?

abort() does usually generate a useful core dump/stack backtrace IME.

___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


Re: [vdr] Logs from building VDR 2.1.3 with Clang 3.4.1

2014-02-08 Thread Klaus Schmidinger

On 08.02.2014 14:34, Tony Houghton wrote:

On Sat, 08 Feb 2014 13:51:10 +0100
Klaus Schmidinger  wrote:


  channels.c:45:119: warning: data argument not used by format string 
[-Wformat-extra-args]
snprintf(buffer, sizeof(buffer), rid ? "%s-%d-%d-%d-%d" : 
"%s-%d-%d-%d", *cSource::ToString(source), nid, tid, sid, rid);
  
~ ^


This is explicitly checked with 'rid ? ...', so the warning is
unjustified (although the compiler probably has a hard time figuring
that out ;-).


The warning is justified, because if rid is 0 it's still there as an
argument, but just happens to have a value of 0. I think you can make
snprintf "consume" it without printing anything by adding %.d to the
second format string.


I'm afraid not.
If I run

#include 
int main(void)
{
  for (int n = 0; n < 2; n++)
  printf(n ? "'%d-%d'\n" : "'%d%.d'\n", 1, 2);
  return 0;
}

I get

'12'
'1-2'

But maybe there *is* such a format character, it just isn't "%.d".


  eit.c:223:43: warning: comparison of constant 176 with expression of 
type 'SI::LinkageType' is always false
[-Wtautological-constant-out-of-range-compare]
   if (ld->getLinkageType() == 0xB0) { // Premiere World
    ^  


I assume this is because the enum LinkageType doesn't contain 0xB0.
However, the actual value that comes from the SI data may well be
0xB0, so I'm now typecasting uint(ld->getLinkageType()).


If 0xB0 has a significant meaning wouldn't it be a good idea to add it
to the enum?


You're probably right. Since there are already several "Premiere" related 
definitions in
libsi/si.h I guess it won't hurt to add yet another one. The DVB standard 
should never have
allowed all this "private" stuff...


  receiver.c:28:6: warning: indirection of non-volatile null pointer 
will be deleted, not trap [-Wnull-dereference]
   *(char *)0 = 0; // cause a segfault
   ^~
  receiver.c:28:6: note: consider using __builtin_trap() or qualifying 
pointer with 'volatile'


Can you suggest a different way of causing a segfault at this point?


You could add volatile as the warning suggests. Is there a good reason
not to use abort() instead?


The idea behind this was to allow for easy backtracking in such a case.
I believe abort() wouldn't allow this, would it?

Klaus

___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


Re: [vdr] Logs from building VDR 2.1.3 with Clang 3.4.1

2014-02-08 Thread Tony Houghton
On Sat, 08 Feb 2014 13:51:10 +0100
Klaus Schmidinger  wrote:

> >  channels.c:45:119: warning: data argument not used by format 
> > string [-Wformat-extra-args]
> >snprintf(buffer, sizeof(buffer), rid ? "%s-%d-%d-%d-%d" : 
> > "%s-%d-%d-%d", *cSource::ToString(source), nid, tid, sid, rid);
> >  
> > ~ ^
> 
> This is explicitly checked with 'rid ? ...', so the warning is
> unjustified (although the compiler probably has a hard time figuring
> that out ;-).

The warning is justified, because if rid is 0 it's still there as an
argument, but just happens to have a value of 0. I think you can make
snprintf "consume" it without printing anything by adding %.d to the
second format string.

> >  eit.c:223:43: warning: comparison of constant 176 with expression 
> > of type 'SI::LinkageType' is always false
> >[-Wtautological-constant-out-of-range-compare]
> >   if (ld->getLinkageType() == 0xB0) { // Premiere 
> > World
> >    ^  
> 
> I assume this is because the enum LinkageType doesn't contain 0xB0.
> However, the actual value that comes from the SI data may well be
> 0xB0, so I'm now typecasting uint(ld->getLinkageType()).

If 0xB0 has a significant meaning wouldn't it be a good idea to add it
to the enum?

> >  receiver.c:28:6: warning: indirection of non-volatile null pointer 
> > will be deleted, not trap [-Wnull-dereference]
> >   *(char *)0 = 0; // cause a segfault
> >   ^~
> >  receiver.c:28:6: note: consider using __builtin_trap() or 
> > qualifying pointer with 'volatile'
> 
> Can you suggest a different way of causing a segfault at this point?

You could add volatile as the warning suggests. Is there a good reason
not to use abort() instead?

___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr


Re: [vdr] Logs from building VDR 2.1.3 with Clang 3.4.1

2014-02-08 Thread Klaus Schmidinger

On 08.02.2014 09:59, Paul Menzel wrote:

Dear VDR folks,


building VDR 2.1.3 with Clang 3.4.1 the warnings below are shown.

Most warnings look like they can be ignored. Maybe you can spot
something, which should be fixed.

 $ clang --version
 Debian clang version 3.4-1 (tags/RELEASE_34/final) (based on LLVM 3.4)
 Target: i386-pc-linux-gnu
 Thread model: posix
 $ CC=clang CXX=clang++ make
 […]
 clang: warning: treating 'c' input as 'c++' when in C++ mode, this 
behavior is deprecated


You may want to find an option that suppresses this warning, because the file 
names of VDR (*.c)
are not going to change.


 ...
 channels.c:45:119: warning: data argument not used by format string 
[-Wformat-extra-args]
   snprintf(buffer, sizeof(buffer), rid ? "%s-%d-%d-%d-%d" : 
"%s-%d-%d-%d", *cSource::ToString(source), nid, tid, sid, rid);
 
~ ^


This is explicitly checked with 'rid ? ...', so the warning is unjustified 
(although the compiler
probably has a hard time figuring that out ;-).


 ...
 ci.c:867:18: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
  tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...
  ^
  .mjd =
 ci.c:867:36: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
  tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...
^~~
.h =
 ci.c:867:65: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
  tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...
 ^~~
 .m =
 ci.c:867:93: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
  tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...

 ^~~

 .s =
 ci.c:867:121: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
  tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : 
short(htons(tm_lo...

 ^~~~

 .offset =
 ci.c:1007:47: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
  tDisplayReply dr = { id : DRI_MMI_MODE_ACK, 
mode : MM_HIGH_LEVEL };
   ^~~~
   .id =
 ci.c:1007:70: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
  tDisplayReply dr = { id : DRI_MMI_MODE_ACK, 
mode : MM_HIGH_LEVEL };
  
^~
  
.mode =


Fixed in the attached patch.


 ...
 In file included from dvbspu.c:14:
 ./dvbspu.h:104:10: warning: private field 'ready' is not used 
[-Wunused-private-field]
 bool ready;
  ^


Fixed in the attached patch.


 ...
 dvbsubtitle.c:37:13: warning: unused variable 'DebugBitmaps' 
[-Wunused-variable]
 static bool DebugBitmaps   = DebugVerbose || DebugNormal;
 ^


Fixed in the attached patch.


 ...
 eit.c:223:43: warning: comparison of constant 176 with expression of 
type 'SI::LinkageType' is always false
   [-Wtautological-constant-out-of-range-compare]
  if (ld->getLinkageType() == 0xB0) { // Premiere World
   ^  


I assume this is because the enum LinkageType doesn't contain 0xB0. However, 
the actual value that
comes from the SI data may well be 0xB0, so I'm now typecasting 
uint(ld

[vdr] Logs from building VDR 2.1.3 with Clang 3.4.1

2014-02-08 Thread Paul Menzel
Dear VDR folks,


building VDR 2.1.3 with Clang 3.4.1 the warnings below are shown.

Most warnings look like they can be ignored. Maybe you can spot
something, which should be fixed.

$ clang --version
Debian clang version 3.4-1 (tags/RELEASE_34/final) (based on LLVM 3.4)
Target: i386-pc-linux-gnu
Thread model: posix
$ CC=clang CXX=clang++ make
[…]
clang: warning: treating 'c' input as 'c++' when in C++ mode, this 
behavior is deprecated
clang++ -g -O3 -Wall -Werror=overloaded-virtual -Wno-parentheses -fPIC 
-c -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-D_LARGEFILE64_SOURCE -DREMOTE_KBD -DLIRC_DEVICE=\"/var/run/lirc/lircd\" 
-DVIDEODIR=\"/srv/vdr/video\" -DCONFDIR=\"/var/lib/vdr\" 
-DCACHEDIR=\"/var/cache/vdr\" -DRESDIR=\"/usr/local/share/vdr\" 
-DPLUGINDIR=\"/usr/local/lib/vdr\" -DLOCDIR=\"/usr/local/share/locale\" 
-I/usr/include/freetype2-o audio.o audio.c
clang: warning: treating 'c' input as 'c++' when in C++ mode, this 
behavior is deprecated
clang++ -g -O3 -Wall -Werror=overloaded-virtual -Wno-parentheses -fPIC 
-c -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-D_LARGEFILE64_SOURCE -DREMOTE_KBD -DLIRC_DEVICE=\"/var/run/lirc/lircd\" 
-DVIDEODIR=\"/srv/vdr/video\" -DCONFDIR=\"/var/lib/vdr\" 
-DCACHEDIR=\"/var/cache/vdr\" -DRESDIR=\"/usr/local/share/vdr\" 
-DPLUGINDIR=\"/usr/local/lib/vdr\" -DLOCDIR=\"/usr/local/share/locale\" 
-I/usr/include/freetype2-o channels.o channels.c
clang: warning: treating 'c' input as 'c++' when in C++ mode, this 
behavior is deprecated
channels.c:45:119: warning: data argument not used by format string 
[-Wformat-extra-args]
  snprintf(buffer, sizeof(buffer), rid ? "%s-%d-%d-%d-%d" : 
"%s-%d-%d-%d", *cSource::ToString(source), nid, tid, sid, rid);

~ ^
1 warning generated.
clang++ -g -O3 -Wall -Werror=overloaded-virtual -Wno-parentheses -fPIC 
-c -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-D_LARGEFILE64_SOURCE -DREMOTE_KBD -DLIRC_DEVICE=\"/var/run/lirc/lircd\" 
-DVIDEODIR=\"/srv/vdr/video\" -DCONFDIR=\"/var/lib/vdr\" 
-DCACHEDIR=\"/var/cache/vdr\" -DRESDIR=\"/usr/local/share/vdr\" 
-DPLUGINDIR=\"/usr/local/lib/vdr\" -DLOCDIR=\"/usr/local/share/locale\" 
-I/usr/include/freetype2-o ci.o ci.c
clang: warning: treating 'c' input as 'c++' when in C++ mode, this 
behavior is deprecated
ci.c:867:18: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
 tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...
 ^
 .mjd = 
ci.c:867:36: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
 tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...
   ^~~
   .h = 
ci.c:867:65: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
 tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...
^~~
.m = 
ci.c:867:93: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
 tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : short(htons(...

^~~

.s = 
ci.c:867:121: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
 tTime T = { mjd : htons(MJD), h : DEC2BCD(tm_gmt.tm_hour), m : 
DEC2BCD(tm_gmt.tm_min), s : DEC2BCD(tm_gmt.tm_sec), offset : 
short(htons(tm_lo...

^~~~

.offset = 
ci.c:1007:47: warning: use of GNU old-style field designator extension 
[-Wgnu-designator]
 tDisplayReply dr = { id : DRI_MMI_MODE_ACK, 
mode : MM_HIGH_LEVEL };
  ^~~~
  .id = 
ci.c:1007:70: warning: use of GNU old-st