On 05.12.22 16:54, Marko Mäkelä wrote:
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.
In cDevice::GetDevice() SlotPriority[] is never touched if NumCamSlots is 0.
So the compiler may assume whatever it wants in that case, it won't matter.
Or can you show a case where it actually misbehaves?
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.
Doing this would never select a device for FTA channels.
...
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); }
...
};
cDvbPlayerControl is the class that creates and deletes the player.
cControl is only given the player to control it in an abstract manner.
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.
Well, IMHO whoever implemented such an "optimization" should be banned from programming for life!
This is not an optimization, it's an insidious TRAP!
The man page on memcpy() doesn't say that the size can't be 0.
...
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?
Sorry, my bad. I missed that pfd is passed to poll()
Please try this:
--- ./sections.c 2022/01/31 21:21:42 5.3
+++ ./sections.c 2022/12/05 22:46:24
@@ -180,6 +180,11 @@
startFilters = false;
}
int NumFilters = filterHandles.Count();
+ if (NumFilters == 0) {
+ Unlock();
+ cCondWait::SleepMs(100);
+ continue;
+ }
pollfd pfd[NumFilters];
for (cFilterHandle *fh = filterHandles.First(); fh; fh = filterHandles.Next(fh)) {
int i = fh->Index();
Klaus
_______________________________________________
vdr mailing list
vdr@xxxxxxxxxxx
https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
