Re: [vdr] cppcheck: VDR 1.7.18: [timers.c:53]: (error) snprintf size is out of bounds

[Klaus Schmidinger]

On 15.06.2011 19:37, Udo Richter wrote:

Am 15.06.2011 18:34, schrieb Klaus Schmidinger:

On 15.06.2011 15:30, Paul Menzel wrote:

if (Instant&&   channel)
   snprintf(file, sizeof(file), "%s%s",
Setup.MarkInstantRecord ? "@" : "", *Setup.NameInstantRecord ?
Setup.NameInstantRecord : channel->Name());


...this should be

   sizeof(file) - 1


Actually, all versions of snprintf documentation I've just checked agree
that snprintf will write at most size-1 chars and a trailing 0 byte, so
it was ok before too. But for safety, on byte less doesn't hurt.


Gee, you're right!


Or is there some broken implementation out there that may write beyond
str[size-1]?


None that I know of.

Well, since the docs for snprintf are clear about this, let's
leave things as they are.

I wonder, though, why cppcheck thinks there's something wrong
here...

Klaus

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

Re: [vdr] cppcheck: VDR 1.7.18: [timers.c:53]: (error) snprintf size is out of bounds

[Gerald Dachs]

Am Wed, 15 Jun 2011 18:34:59 +0200
schrieb Klaus Schmidinger :

> ...this should be
> 
>sizeof(file) - 1
> 
> Thanks for the bug report.

This is no bug. The size parameter includes the '\0' byte.

Gerald

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

Re: [vdr] cppcheck: VDR 1.7.18: [timers.c:53]: (error) snprintf size is out of bounds

[Udo Richter]

Am 15.06.2011 18:34, schrieb Klaus Schmidinger:
> On 15.06.2011 15:30, Paul Menzel wrote:
>>if (Instant&&  channel)
>>   snprintf(file, sizeof(file), "%s%s",
>> Setup.MarkInstantRecord ? "@" : "", *Setup.NameInstantRecord ?
>> Setup.NameInstantRecord : channel->Name());
> 
> ...this should be
> 
>   sizeof(file) - 1

Actually, all versions of snprintf documentation I've just checked agree
that snprintf will write at most size-1 chars and a trailing 0 byte, so
it was ok before too. But for safety, on byte less doesn't hurt.

Or is there some broken implementation out there that may write beyond
str[size-1]?

(strncpy is more broken, thats why my typical usage is:
  strncpy(dest, src, sizeof(dest)-1);
  dest[sizeof(dest)-1] = 0;
)

Cheers,

Udo

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

Re: [vdr] cppcheck: VDR 1.7.18: [timers.c:53]: (error) snprintf size is out of bounds

[Klaus Schmidinger]

On 15.06.2011 15:30, Paul Menzel wrote:

Dear VDR folks,


Cppcheck 1.49 [1] reports the following error when run against VDR
1.7.18.

[timers.c:53]: (error) snprintf size is out of bounds

Looking at `timers.c` in `CTimer` `*file = 0` and afterward written to
if I am not mistaken.


This just sets the string to be "empty", but...


 […]
 cTimer::cTimer(bool Instant, bool Pause, cChannel *Channel)
 {
   ...
   *file = 0;
   aux = NULL;
   event = NULL;
   if (Instant&&  channel)
  snprintf(file, sizeof(file), "%s%s", Setup.MarkInstantRecord ? "@" : 
"", *Setup.NameInstantRecord ? Setup.NameInstantRecord : channel->Name());


...this should be

  sizeof(file) - 1

Thanks for the bug report.

Klaus


 […]

Unfortunately I do not know C++ well enough to judge this error message.


Thanks,

Paul


[1] http://cppcheck.sourceforge.net/


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

[vdr] cppcheck: VDR 1.7.18: [timers.c:53]: (error) snprintf size is out of bounds

[Paul Menzel]

Dear VDR folks,


Cppcheck 1.49 [1] reports the following error when run against VDR
1.7.18.

[timers.c:53]: (error) snprintf size is out of bounds

Looking at `timers.c` in `CTimer` `*file = 0` and afterward written to
if I am not mistaken.

[…]
cTimer::cTimer(bool Instant, bool Pause, cChannel *Channel)
{
  startTime = stopTime = 0;
  lastSetEvent = 0;
  recording = pending = inVpsMargin = false;
  flags = tfNone;
  if (Instant)
 SetFlags(tfActive | tfInstant);
  channel = Channel ? Channel : 
Channels.GetByNumber(cDevice::CurrentChannel());
  time_t t = time(NULL);
  struct tm tm_r;
  struct tm *now = localtime_r(&t, &tm_r);
  day = SetTime(t, 0);
  weekdays = 0;
  start = now->tm_hour * 100 + now->tm_min;
  stop = now->tm_hour * 60 + now->tm_min + Setup.InstantRecordTime;
  stop = (stop / 60) * 100 + (stop % 60);
  if (stop >= 2400)
 stop -= 2400;
  priority = Pause ? Setup.PausePriority : Setup.DefaultPriority;
  lifetime = Pause ? Setup.PauseLifetime : Setup.DefaultLifetime;
  *file = 0;
  aux = NULL;
  event = NULL;
  if (Instant && channel)
 snprintf(file, sizeof(file), "%s%s", Setup.MarkInstantRecord ? "@" 
: "", *Setup.NameInstantRecord ? Setup.NameInstantRecord : channel->Name());
[…]

Unfortunately I do not know C++ well enough to judge this error message.


Thanks,

Paul


[1] http://cppcheck.sourceforge.net/


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