Hi Klaus,
Mon, Dec 05, 2022 at 04:08:45PM +0100, Klaus Schmidinger wrote:
If NumCamSlots is 0, SlotPriority[] is never accessed.
So why allocate memory for it if it is never used?
Allocating a variable-length array of length 0 is undefined behaviour.
The compiler is allowed to assume NumCamSlots>0 and optimize something
based on that assumption.
You are right, it would be better to treat NumCamSlots==0 as a special
case. I only tried adding a "return NULL", which resulted in an error
message that a channel is unavailable, for a free-to-view channel.
Instead if typecasting I guess I'll rather do it this way:
--- ./dvbplayer.c 2022/01/13 21:41:41 5.1
+++ ./dvbplayer.c 2022/12/05 14:29:50
@@ -981,8 +981,10 @@
// --- cDvbPlayerControl -----------------------------------------------------
cDvbPlayerControl::cDvbPlayerControl(const char *FileName, bool PauseLive)
-:cControl(player = new cDvbPlayer(FileName, PauseLive))
+:cControl(NULL, PauseLive)
{
+ player = new cDvbPlayer(FileName, PauseLive);
+ SetPlayer(player);
}
Sure, that would work too. In my C++ projects, I try to declare as many
data members as possible as const, and initialize them at object
creation time. As far as I understand, static_cast is completely clean
here.
Related question: Do we need to duplicate cControl::player in
cDvbPlayerControl::player? Perhaps there could be a member function that
returns the protected data member of the base class:
class cDvbPlayerControl {
...
private:
cDvbPlayer *GetPlayer() const { return static_cast<cDvbPlayer*>(player); }
...
};
If (rows * pitch) is 0, nothing is copied.
Why the extra check?
Because invoking memcpy() with null pointers is undefined behaviour, the
compiler is allowed to assume that both pointers are nonnull, and
allowed to optimize subsequent checks based on that assumption. Because
this member function is inlined, the assumption could propagate to lots
of other code.
Basically, for code like this:
void copy_and_set(char *a, const char *b, size_t size)
{
memcpy(a, b, size);
if (a)
*a = 1;
}
the compiler is allowed to optimize away the "if (a)" check.
Some years ago, I witnessed this in another codebase, when it was
compiled with a new enough GCC and -O2. It was quite a head-scratcher,
because the memcpy() or memset() call was located far away from the
place where the surprising optimization took place.
If x2 ever becomes negative, something else must have gone wrong. So I
think this check here is moot.
I tend to agree. I have seen examples of that in an interpreter. It
would first invoke some undefined-behaviour arithmetics, and then the
compiler would optimize away an overflow check that was made later.
I will try to provide a stack trace for this, to see where the garbage
was coming from, so that this bug can be fixed closer to its source.
If NumFilters is 0, pfd[] is never accessed.
So why allocate memory for it if it is never used?
Could "if (NumFilters == 0)" be added to skip the allocation and the
subsequent code? On a quick read of "man 2 poll", I did not find any
mention on what it should do if the array is empty, nor did I check what
it would actually do: Wait for the timeout, or return immediately?
Marko
_______________________________________________
vdr mailing list
vdr@xxxxxxxxxxx
https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
