Re: [PATCH] Fix undefined behaviour

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tue, Dec 06, 2022 at 01:24:09PM +0200, Marko Mäkelä wrote:
The first attached patch includes your suggested fixes and nothing that you opposed so far. The second attached patch fixes the following 2 issues. I agree that the NumCamSlots==0 case could be solved in a nicer way.

I tried to make sense out of the generated code, and I did not find any evidence of a subsequent unsafe optimization. Still, I think that it is a good practice to address any compiler warnings even when they are genuinely bogus, because often such warnings can identify real trouble. The only question should be whether silencing the warnings could introduce an unacceptable amount of runtime overhead.

Maybe the simplest way to silence the warning would be to bloat the variable-length array with 1 extra element, wasting sizeof(int) bytes of stack space:

  int SlotPriority[NumCamSlots + 1];

That would avoid adding a conditional branch, like the one that would have been part of my initially suggested std::max(NumCamSlots, 1). It might not even translate into an extra machine instruction, if the constant can be embedded in an instruction that would be generated anyway.

I tested the following alternative patch. It is duplicating a lot of source code, which I usually find a bad idea, including in this case. Some "imp <<= 1" or "imp <<= 8" are unnecessary and could be omitted. I retained the statements to keep the transformed code more similar to what it was copied and adapted from.

	Marko
diff --git a/device.c b/device.c
index 4b9c9cc7..1c1b4d28 100644
--- a/device.c
+++ b/device.c
@@ -249,6 +249,57 @@ cDevice *cDevice::GetDevice(const cChannel *Channel, int Priority, bool LiveView
 {
   // Collect the current priorities of all CAM slots that can decrypt the channel:
   int NumCamSlots = CamSlots.Count();
+  if (!NumCamSlots) {
+     bool NeedsDetachReceivers = false;
+     const bool InternalCamNeeded = Channel->Ca() >= CA_ENCRYPTED_MIN;
+     cDevice *d = NULL;
+     uint32_t Impact = 0xFFFFFFFF; // we're looking for a device with the least impact
+     for (int i = 0; i < numDevices; i++) {
+         if (Channel->Ca() && Channel->Ca() <= CA_DVB_MAX && Channel->Ca() != device[i]->DeviceNumber() + 1)
+            continue; // a specific card was requested, but not this one
+         const bool HasInternalCam = device[i]->HasInternalCam();
+         if (InternalCamNeeded && !HasInternalCam)
+            continue; // no CAM is able to decrypt this channel and the device uses vdr handled CAMs
+         bool ndr;
+         if (device[i]->ProvidesChannel(Channel, Priority, &ndr)) { // this device is basically able to do the job
+            // Put together an integer number that reflects the "impact" using
+            // this device would have on the overall system. Each condition is represented
+            // by one bit in the number (or several bits, if the condition is actually
+            // a numeric value). The sequence in which the conditions are listed corresponds
+            // to their individual severity, where the one listed first will make the most
+            // difference, because it results in the most significant bit of the result.
+            uint32_t imp = 0;
+            imp <<= 1;
+            imp <<= 1; imp |= LiveView && (!device[i]->IsPrimaryDevice() || ndr);                                  // prefer the primary device for live viewing if we don't need to detach existing receivers
+            imp <<= 1; imp |= !device[i]->Receiving() && (device[i] != cTransferControl::ReceiverDevice() || device[i]->IsPrimaryDevice()) || ndr; // use receiving devices if we don't need to detach existing receivers, but avoid primary device in local transfer mode
+            imp <<= 1; imp |= device[i]->Receiving();                                                               // avoid devices that are receiving
+            imp <<= 5; imp |= GetClippedNumProvidedSystems(5, device[i]) - 1;                                       // avoid cards which support multiple delivery systems
+            imp <<= 1; imp |= device[i] == cTransferControl::ReceiverDevice();                                      // avoid the Transfer Mode receiver device
+            imp <<= 8; imp |= device[i]->Priority() - IDLEPRIORITY;                                                 // use the device with the lowest priority (- IDLEPRIORITY to assure that values -100..99 can be used)
+            imp <<= 8;
+            imp <<= 1; imp |= ndr;                                                                                  // avoid devices if we need to detach existing receivers
+            imp <<= 1; imp |= !InternalCamNeeded && device[i]->HasCi();                       // avoid cards with Common Interface for FTA channels
+            imp <<= 1; imp |= device[i]->AvoidRecording();                                                          // avoid SD full featured cards
+            imp <<= 1;
+            imp <<= 1; imp |= device[i]->IsPrimaryDevice();                                                         // avoid the primary device
+            if (imp < Impact) {
+               // This device has less impact than any previous one, so we take it.
+               Impact = imp;
+               d = device[i];
+               NeedsDetachReceivers = ndr;
+               }
+            }
+         }
+
+     if (d) {
+        if (!Query && NeedsDetachReceivers)
+           d->DetachAllReceivers();
+        if (d->CamSlot() && !d->CamSlot()->IsDecrypting())
+           d->CamSlot()->Assign(NULL);
+        }
+     return d;
+     }
+
   int SlotPriority[NumCamSlots];
   int NumUsableSlots = 0;
   bool InternalCamNeeded = false;
_______________________________________________
vdr mailing list
vdr@xxxxxxxxxxx
https://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr

[Index of Archives]     [Linux Media]     [Asterisk]     [DCCP]     [Netdev]     [Xorg]     [Util Linux NG]     [Xfree86]     [Big List of Linux Books]     [Fedora Users]     [Fedora Women]     [ALSA Devel]     [Linux USB]

  Powered by Linux