* [patch] caif: cleanup: remove duplicate checks
@ 2010-05-22 20:45 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2010-05-22 20:45 UTC (permalink / raw)
To: netdev; +Cc: Sjur Braendeland, Stephen Rothwell, kernel-janitors,
David S. Miller
These checks merely duplicate the things we've asserted already. In the
case of the checks for null we've already dereferenced those variables
as well.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index df43f26..cc2f072 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -313,14 +313,10 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
caif_assert(phyinfo->phy_layer != NULL);
caif_assert(phyinfo->phy_layer->id = phyid);
- if (phyinfo != NULL &&
- phyinfo->phy_ref_count++ = 0 &&
- phyinfo->phy_layer != NULL &&
+ if (phyinfo->phy_ref_count++ = 0 &&
phyinfo->phy_layer->modemcmd != NULL) {
- caif_assert(phyinfo->phy_layer->id = phyid);
phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
_CAIF_MODEMCMD_PHYIF_USEFULL);
-
}
adapt_layer->id = channel_id;
^ permalink raw reply related [flat|nested] 10+ messages in thread* [patch] caif: cleanup: remove duplicate checks
@ 2010-05-22 20:45 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2010-05-22 20:45 UTC (permalink / raw)
To: netdev; +Cc: Sjur Braendeland, Stephen Rothwell, kernel-janitors,
David S. Miller
These checks merely duplicate the things we've asserted already. In the
case of the checks for null we've already dereferenced those variables
as well.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index df43f26..cc2f072 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -313,14 +313,10 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
caif_assert(phyinfo->phy_layer != NULL);
caif_assert(phyinfo->phy_layer->id == phyid);
- if (phyinfo != NULL &&
- phyinfo->phy_ref_count++ == 0 &&
- phyinfo->phy_layer != NULL &&
+ if (phyinfo->phy_ref_count++ == 0 &&
phyinfo->phy_layer->modemcmd != NULL) {
- caif_assert(phyinfo->phy_layer->id == phyid);
phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
_CAIF_MODEMCMD_PHYIF_USEFULL);
-
}
adapt_layer->id = channel_id;
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [patch] caif: cleanup: remove duplicate checks
2010-05-22 20:45 ` Dan Carpenter
@ 2010-05-23 8:27 ` walter harms
-1 siblings, 0 replies; 10+ messages in thread
From: walter harms @ 2010-05-23 8:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: netdev, Sjur Braendeland, Stephen Rothwell, kernel-janitors,
David S. Miller
Dan Carpenter schrieb:
> These checks merely duplicate the things we've asserted already. In the
> case of the checks for null we've already dereferenced those variables
> as well.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
> index df43f26..cc2f072 100644
> --- a/net/caif/cfcnfg.c
> +++ b/net/caif/cfcnfg.c
> @@ -313,14 +313,10 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
> caif_assert(phyinfo->phy_layer != NULL);
> caif_assert(phyinfo->phy_layer->id = phyid);
>
> - if (phyinfo != NULL &&
> - phyinfo->phy_ref_count++ = 0 &&
> - phyinfo->phy_layer != NULL &&
> + if (phyinfo->phy_ref_count++ = 0 &&
> phyinfo->phy_layer->modemcmd != NULL) {
> - caif_assert(phyinfo->phy_layer->id = phyid);
> phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
> _CAIF_MODEMCMD_PHYIF_USEFULL);
> -
> }
> adapt_layer->id = channel_id;
>
hi Dan,
the phyinfo->phy_ref_count++ is very hidden, perhaps in terms of readability
it is better to move the phyinfo->phy_ref_count++ inside.
just my 2 cents,
re,
wh
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] caif: cleanup: remove duplicate checks
@ 2010-05-23 8:27 ` walter harms
0 siblings, 0 replies; 10+ messages in thread
From: walter harms @ 2010-05-23 8:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: netdev, Sjur Braendeland, Stephen Rothwell, kernel-janitors,
David S. Miller
Dan Carpenter schrieb:
> These checks merely duplicate the things we've asserted already. In the
> case of the checks for null we've already dereferenced those variables
> as well.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
>
> diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
> index df43f26..cc2f072 100644
> --- a/net/caif/cfcnfg.c
> +++ b/net/caif/cfcnfg.c
> @@ -313,14 +313,10 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
> caif_assert(phyinfo->phy_layer != NULL);
> caif_assert(phyinfo->phy_layer->id == phyid);
>
> - if (phyinfo != NULL &&
> - phyinfo->phy_ref_count++ == 0 &&
> - phyinfo->phy_layer != NULL &&
> + if (phyinfo->phy_ref_count++ == 0 &&
> phyinfo->phy_layer->modemcmd != NULL) {
> - caif_assert(phyinfo->phy_layer->id == phyid);
> phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
> _CAIF_MODEMCMD_PHYIF_USEFULL);
> -
> }
> adapt_layer->id = channel_id;
>
hi Dan,
the phyinfo->phy_ref_count++ is very hidden, perhaps in terms of readability
it is better to move the phyinfo->phy_ref_count++ inside.
just my 2 cents,
re,
wh
^ permalink raw reply [flat|nested] 10+ messages in thread* [patch v2] caif: cleanup: remove duplicate checks
2010-05-23 8:27 ` walter harms
@ 2010-05-24 16:29 ` Dan Carpenter
-1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2010-05-24 16:29 UTC (permalink / raw)
To: walter harms
Cc: netdev, Sjur Braendeland, Stephen Rothwell, kernel-janitors,
David S. Miller
"phyinfo" can never be null here because we assigned it an address, so I
removed both the assert and the second check inside the if statement. I
removed the "phyinfo->phy_layer != NULL" check as well because that was
asserted earlier.
Walter Harms suggested I move the "phyinfo->phy_ref_count++;" outside
the if condition for readability, so I have done that.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
The original code increments phyinfo->phy_ref_count++ even if
phyinfo->phy_layer->modemcmd is NULL. I kept it the same in my code,
but maybe I should change that?
v2: Removed an assert. Moved the increment outside the if condition.
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index df43f26..7c81974 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -308,19 +308,15 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
caif_assert(cnfg != NULL);
caif_assert(phyid != 0);
phyinfo = &cnfg->phy_layers[phyid];
- caif_assert(phyinfo != NULL);
caif_assert(phyinfo->id = phyid);
caif_assert(phyinfo->phy_layer != NULL);
caif_assert(phyinfo->phy_layer->id = phyid);
- if (phyinfo != NULL &&
- phyinfo->phy_ref_count++ = 0 &&
- phyinfo->phy_layer != NULL &&
+ phyinfo->phy_ref_count++;
+ if (phyinfo->phy_ref_count = 1 &&
phyinfo->phy_layer->modemcmd != NULL) {
- caif_assert(phyinfo->phy_layer->id = phyid);
phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
_CAIF_MODEMCMD_PHYIF_USEFULL);
-
}
adapt_layer->id = channel_id;
^ permalink raw reply related [flat|nested] 10+ messages in thread* [patch v2] caif: cleanup: remove duplicate checks
@ 2010-05-24 16:29 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2010-05-24 16:29 UTC (permalink / raw)
To: walter harms
Cc: netdev, Sjur Braendeland, Stephen Rothwell, kernel-janitors,
David S. Miller
"phyinfo" can never be null here because we assigned it an address, so I
removed both the assert and the second check inside the if statement. I
removed the "phyinfo->phy_layer != NULL" check as well because that was
asserted earlier.
Walter Harms suggested I move the "phyinfo->phy_ref_count++;" outside
the if condition for readability, so I have done that.
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
The original code increments phyinfo->phy_ref_count++ even if
phyinfo->phy_layer->modemcmd is NULL. I kept it the same in my code,
but maybe I should change that?
v2: Removed an assert. Moved the increment outside the if condition.
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index df43f26..7c81974 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -308,19 +308,15 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
caif_assert(cnfg != NULL);
caif_assert(phyid != 0);
phyinfo = &cnfg->phy_layers[phyid];
- caif_assert(phyinfo != NULL);
caif_assert(phyinfo->id == phyid);
caif_assert(phyinfo->phy_layer != NULL);
caif_assert(phyinfo->phy_layer->id == phyid);
- if (phyinfo != NULL &&
- phyinfo->phy_ref_count++ == 0 &&
- phyinfo->phy_layer != NULL &&
+ phyinfo->phy_ref_count++;
+ if (phyinfo->phy_ref_count == 1 &&
phyinfo->phy_layer->modemcmd != NULL) {
- caif_assert(phyinfo->phy_layer->id == phyid);
phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
_CAIF_MODEMCMD_PHYIF_USEFULL);
-
}
adapt_layer->id = channel_id;
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [patch v2] caif: cleanup: remove duplicate checks
2010-05-24 16:29 ` Dan Carpenter
@ 2010-05-24 20:47 ` Sjur Brændeland
-1 siblings, 0 replies; 10+ messages in thread
From: Sjur Brændeland @ 2010-05-24 20:47 UTC (permalink / raw)
To: Dan Carpenter
Cc: walter harms, netdev, Sjur Braendeland, Stephen Rothwell,
kernel-janitors, David S. Miller
Dan Carpenter <error27@gmail.com> wrote:
> "phyinfo" can never be null here because we assigned it an address, so I
> removed both the assert and the second check inside the if statement. Â I
> removed the "phyinfo->phy_layer != NULL" check as well because that was
> asserted earlier.
>
> Walter Harms suggested I move the "phyinfo->phy_ref_count++;" outside
> the if condition for readability, so I have done that.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Thanks, this looks good.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch v2] caif: cleanup: remove duplicate checks
@ 2010-05-24 20:47 ` Sjur Brændeland
0 siblings, 0 replies; 10+ messages in thread
From: Sjur Brændeland @ 2010-05-24 20:47 UTC (permalink / raw)
To: Dan Carpenter
Cc: walter harms, netdev, Sjur Braendeland, Stephen Rothwell,
kernel-janitors, David S. Miller
Dan Carpenter <error27@gmail.com> wrote:
> "phyinfo" can never be null here because we assigned it an address, so I
> removed both the assert and the second check inside the if statement. I
> removed the "phyinfo->phy_layer != NULL" check as well because that was
> asserted earlier.
>
> Walter Harms suggested I move the "phyinfo->phy_ref_count++;" outside
> the if condition for readability, so I have done that.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Thanks, this looks good.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch v2] caif: cleanup: remove duplicate checks
2010-05-24 20:47 ` Sjur Brændeland
@ 2010-05-31 7:32 ` David Miller
-1 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-05-31 7:32 UTC (permalink / raw)
To: sjurbren; +Cc: error27, wharms, netdev, sjur.brandeland, sfr, kernel-janitors
From: Sjur Brændeland <sjurbren@gmail.com>
Date: Mon, 24 May 2010 22:47:41 +0200
> Dan Carpenter <error27@gmail.com> wrote:
>> "phyinfo" can never be null here because we assigned it an address, so I
>> removed both the assert and the second check inside the if statement. I
>> removed the "phyinfo->phy_layer != NULL" check as well because that was
>> asserted earlier.
>>
>> Walter Harms suggested I move the "phyinfo->phy_ref_count++;" outside
>> the if condition for readability, so I have done that.
>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
> Acked-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
> Thanks, this looks good.
Applied.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch v2] caif: cleanup: remove duplicate checks
@ 2010-05-31 7:32 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-05-31 7:32 UTC (permalink / raw)
To: sjurbren; +Cc: error27, wharms, netdev, sjur.brandeland, sfr, kernel-janitors
From: Sjur Brændeland <sjurbren@gmail.com>
Date: Mon, 24 May 2010 22:47:41 +0200
> Dan Carpenter <error27@gmail.com> wrote:
>> "phyinfo" can never be null here because we assigned it an address, so I
>> removed both the assert and the second check inside the if statement. I
>> removed the "phyinfo->phy_layer != NULL" check as well because that was
>> asserted earlier.
>>
>> Walter Harms suggested I move the "phyinfo->phy_ref_count++;" outside
>> the if condition for readability, so I have done that.
>>
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
> Acked-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
> Thanks, this looks good.
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-05-31 7:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-22 20:45 [patch] caif: cleanup: remove duplicate checks Dan Carpenter
2010-05-22 20:45 ` Dan Carpenter
2010-05-23 8:27 ` walter harms
2010-05-23 8:27 ` walter harms
2010-05-24 16:29 ` [patch v2] " Dan Carpenter
2010-05-24 16:29 ` Dan Carpenter
2010-05-24 20:47 ` Sjur Brændeland
2010-05-24 20:47 ` Sjur Brændeland
2010-05-31 7:32 ` David Miller
2010-05-31 7:32 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.