>>> 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@xxxxxxxx> 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
