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@linuxtv.org
https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr

Reply via email to