| Message ID | alpine.DEB.1.10.0901051437490.27085@gandalf.stny.rr.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 5 Jan 2009, Steven Rostedt wrote: > > Impact: clean up > > The code in process_ver_nack is a little obfuscated. This change > makes it a bit more readable by humans. It removes the complex > if statement and replaces it with a cleaner flow of control. Note, I do not have a sparc compiler at my disposal, so I was not able to compile (or boot) test this change. Here's the original code: static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; if ((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major))) { return ldc_abort(lp); } else { struct ldc_packet *p; unsigned long new_tail; p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, vap, sizeof(*vap), &new_tail); if (p) return send_tx_packet(lp, p, new_tail); else return ldc_abort(lp); } } And the new code: static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; struct ldc_packet *p; unsigned long new_tail; if (vp->major == 0 && vp->minor == 0) return ldc_abort(lp); vap = find_by_major(vp->major); if (!vap) return ldc_abort(lp); p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, vap, sizeof(*vap), &new_tail); if (!p) return ldc_abort(lp); return send_tx_packet(lp, p, new_tail); } This should be binary the same. But as I said, I do not have a sparc compiler to test. Thanks, -- Steve -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 5 Jan 2009, Steven Rostedt wrote: > > Impact: clean up > > The code in process_ver_nack is a little obfuscated. This change > makes it a bit more readable by humans. It removes the complex > if statement and replaces it with a cleaner flow of control. > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> Sam, Can you test to see if this patch makes the issue go away. Obviously, Dave would need to be the one to pull it in, or at least ack it. Thanks, -- Steve -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 05, 2009 at 02:56:58PM -0500, Steven Rostedt wrote: > > On Mon, 5 Jan 2009, Steven Rostedt wrote: > > > > > Impact: clean up > > > > The code in process_ver_nack is a little obfuscated. This change > > makes it a bit more readable by humans. It removes the complex > > if statement and replaces it with a cleaner flow of control. > > > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> > > Sam, > > Can you test to see if this patch makes the issue go away. Obviously, Dave > would need to be the one to pull it in, or at least ack it. As expected the patch silence the warning. The warning only triggers when we have an assignment after a boolean || inside an if condition. Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 05, 2009 at 02:42:59PM -0500, Steven Rostedt wrote: > > Impact: clean up > > The code in process_ver_nack is a little obfuscated. This change > makes it a bit more readable by humans. It removes the complex > if statement and replaces it with a cleaner flow of control. > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> Reviewed-by: Sam Ravnborg <sam@ravnborg.org> And I can confirm the warning has dismissed with this patch. Sam -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sam Ravnborg <sam@ravnborg.org> Date: Mon, 5 Jan 2009 21:08:48 +0100 > On Mon, Jan 05, 2009 at 02:42:59PM -0500, Steven Rostedt wrote: > > > > Impact: clean up > > > > The code in process_ver_nack is a little obfuscated. This change > > makes it a bit more readable by humans. It removes the complex > > if statement and replaces it with a cleaner flow of control. > > > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > And I can confirm the warning has dismissed with this patch. Applied, thanks everyone. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sparc/kernel/ldc.c b/arch/sparc/kernel/ldc.c index d689823..6ce5d25 100644 --- a/arch/sparc/kernel/ldc.c +++ b/arch/sparc/kernel/ldc.c @@ -625,22 +625,23 @@ static int process_ver_ack(struct ldc_channel *lp, struct ldc_version *vp) static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; + struct ldc_packet *p; + unsigned long new_tail; - if ((vp->major == 0 && vp->minor == 0) || - !(vap = find_by_major(vp->major))) { + if (vp->major == 0 && vp->minor == 0) + return ldc_abort(lp); + + vap = find_by_major(vp->major); + if (!vap) return ldc_abort(lp); - } else { - struct ldc_packet *p; - unsigned long new_tail; - p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, + p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, vap, sizeof(*vap), &new_tail); - if (p) - return send_tx_packet(lp, p, new_tail); - else - return ldc_abort(lp); - } + if (!p) + return ldc_abort(lp); + + return send_tx_packet(lp, p, new_tail); } static int process_version(struct ldc_channel *lp,
Impact: clean up The code in process_ver_nack is a little obfuscated. This change makes it a bit more readable by humans. It removes the complex if statement and replaces it with a cleaner flow of control. Signed-off-by: Steven Rostedt <srostedt@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html