Re: [PATCH 3/3] move wifi activation fails logging to proper block

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/15/2011 11:55 AM, Brian C. Lane wrote:
On Tue, Nov 15, 2011 at 12:58:08PM +0100, Vratislav Podzimek wrote:
On Mon, 2011-11-14 at 09:48 -0800, Brian C. Lane wrote:
On Fri, Nov 11, 2011 at 01:38:14PM +0100, Vratislav Podzimek wrote:
Otherwise we have 'wifi activation failed' log message even if there's no wifi-related line in kickstart.
---
  loader/kickstart.c |    5 ++---
  1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/loader/kickstart.c b/loader/kickstart.c
index b038406..12082b1 100644
--- a/loader/kickstart.c
+++ b/loader/kickstart.c
@@ -671,11 +671,10 @@ int process_kickstart_wifi (struct loaderData_s * loaderData) {
                      WIFI_PROTECTION_UNPROTECTED, NULL, loaderData->ipinfo_set, loaderData->ipv4,
                      loaderData->gateway, loaderData->dns, loaderData->netmask);
          }
+        if (rc == WIFI_ACTIVATION_OK) loaderData->netDev_set = 1;
+        else logMessage(ERROR, "wifi activation in kickstart failed");

Please split the if/else into separate lines, it makes it more readable.
o.k., no problem, but have a look at
http://www.redhat.com/archives/anaconda-devel-list/2011-May/msg00183.html where pjones argued against this style. Time to make an Anaconda Conding Style page?


I don't think that's what he was complaining about. When dealing with if
clauses that only have 1 statement in their block it should look like
this:

if (rc == WIFI_ACTIVATION_OK)
     loaderData->netDev_set = 1;
else
     logMessage(ERROR, "wifi activation in kickstart failed");

I would also be happy with brackets, but only when they're on the same
line, like this:

if (rc == WIFI_ACTIVATION_OK) {
     loaderData->netDev_set = 1;
} else {
     logMessage(ERROR, "wifi activation in kickstart failed");
}

If there is more than one statement in the block then brackets should
look like the above. In your original patch you had some that had:

}
else {

Which (in my opinion of course), is ugly and a waste of space.

function declarations should also have the { on the same line, although
I see in our loader code we haven't always followed that style.

Thanks for the clarification, Brian.

Vratislav, I think a coding style page on the Anaconda wiki would be a good idea. We have a common style accepted by the team, but we do not have it documented anywhere. We also have a habit of breaking the style but only when we are trying to maintain consistency with existing code. Some things that should be explained on the coding style wiki page:

1) Items about if statements and other blocks, such as Brian explained above. We use 4 space indentation, and so on.

2) If your patch is against an older branch (e.g., rhel5-branch or rhel6-branch), avoid changing any coding style just to bring it up to date with our current coding style.

3) Separate coding style changes from functional changes in patches.

4) In general, coding style change patches should go on the master branch only. Once we branch for a Fedora or RHEL release, we should leave the style there alone.

I'm sure we will have other things to add to the page, but I will wait for a first draft.

--
David Cantrell <dcantrell@xxxxxxxxxx>
Supervisor, Installer Engineering Team
Red Hat, Inc. | Westford, MA | EST5EDT

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list


[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux