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

[Tanu Kaskinen]

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