On Fri, 2013-11-22 at 12:07 -0700, Scott Reeves wrote: > >>> 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) Thanks! Applied. -- Tanu
