[pulseaudio-discuss] [PATCH] Fixed bug 47493 (protocol-native: XOR asserts are expressed in a roundabout way)

[Scott Reeves]

>>> On 11/20/2013 at 12:10 AM, Tanu Kaskinen <tanu.kaskinen at linux.intel.com> wrote:
> On Tue, 2013-11-19 at 11:59 -0900, Parin Porecha wrote:
>> 7 instances of the roundabout asserts were found,
>> All of them have been replaced with appropriate XOR asserts.
>> ---
>>  src/pulsecore/protocol-native.c | 27 +++++++--------------------
>>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> Great, we have a new contributor! The patch is good, I applied it. I
> changed the commit message a bit: I set the "headline" to
> "protocol-native: Express XOR asserts more concisely" and added
> "BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=47493" to the
> end.

This patch happened to randomly catch my eye as I was intrigued to see what appeared to be a duplicate check copy/pasted so many times in the code.
-    CHECK_VALIDITY(c->pstream, idx == PA_INVALID_INDEX || !name, tag, PA_ERR_INVALID);
-    CHECK_VALIDITY(c->pstream, !name || idx == PA_INVALID_INDEX, tag, PA_ERR_INVALID);

After verifying it was a duplicate (the arguments (idx == PA_INVALID_INDEX) and (!name) are idempotent and have no side effects so even the early out logic of the OR makes no difference) a quick scan of the code revealed another one the patch missed.  Here is the trivial patch to remove that ...

>From 677655b50ce35ce95ae34342494689a06d8be473 Mon Sep 17 00:00:00 2001
From: Scott Reeves <sreeves at suse.com>
Date: Thu, 21 Nov 2013 14:43:51 -0700
Subject: [PATCH] protocol-native: Remove written differently but functionally
 redundant check.

---
 src/pulsecore/protocol-native.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index 21a9fe2..86898e8 100644
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -3491,7 +3491,6 @@ static void command_get_info(pa_pdispatch *pd, uint32_t command, uint32_t tag, p
                    command == PA_COMMAND_GET_SOURCE_INFO ||
                    (idx != PA_INVALID_INDEX || name), tag, PA_ERR_INVALID);
     CHECK_VALIDITY(c->pstream, idx == PA_INVALID_INDEX || !name, tag, PA_ERR_INVALID);
-    CHECK_VALIDITY(c->pstream, !name || idx == PA_INVALID_INDEX, tag, PA_ERR_INVALID);

     if (command == PA_COMMAND_GET_SINK_INFO) {
         if (idx != PA_INVALID_INDEX)
-- 
1.8.4