Messages in this thread | | | Date | Wed, 1 Jul 2015 11:35:46 +0200 (CEST) | From | Julia Lawall <> | Subject | Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue |
| |
On Wed, 1 Jul 2015, Sohny Thomas wrote:
> >>>> i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE, > >>>> &scsi.wwnn, NULL); > >>>> - if (i) { > >>>> + if (i) > >>>> return 1; > >>>> - } > >>>> - return 0; > >>>> + else > >>>> + return 0; > >>> No, now this will introduce a new checkpatch warning that "else is not > >>> required after return". why did you introduce this "else"? > >> I did this so that the code is more readable and understandable, I checked and > >> checkpatch didn't call this out , so its clean. > >> > >> Otherwise the above code looks like this > >> > >> if(i) > >> return 1; > >> return 0; > > > > That looks fine. > > > > I haven't looked at the code in detail. Is it normal that the return > > values seem to be 0 1 and -1? Which values represent success and which > > represent an error? It is nicer to have the errors under if and success > > as a direct return at the end. > Here in this driver directory, return 1 means SUCCESS and return 0 means FAILURE
What is -1?
> So you mean my code change is fine?
I think it would be best to have the success case that is not under an if. So if (!i) return 0; return 1;
I guess some day the driver would need more normal error codes?
julia
| |