On Thu, Jan 10, 2019 at 03:23:58PM +0300, Dan Carpenter wrote: > On Thu, Jan 10, 2019 at 06:13:47AM +0000, Sidong Yang wrote: > > Removed unnecessary local variable in have_hgsmi_mode_hints. > > The result of hgsmi_query_conf should be directly compared without > > assigning to local variable. > > > > Signed-off-by: Sidong Yang <realwakka@xxxxxxxxx> > > --- > > I sort of prefer the original... > > The hgsmi_query_conf() function returns negative error codes if it > can't complete the query because of allocation failures. To me that's > more obvious, when we write it in the original way. > > In the new code it looks like it returns bool or something. The > copy_to/from_user() are normally written like if (copy_to_user()) { > but those don't return negative error codes so it's a different > situation. > Hi, Dan. I think you just point out that my code isn't obvious because the function returns negative error codes. I agree with you. But what if change my code like if(hgsmi_query_conf() != 0). > This isn't something in checkpatch or CodingStyle so there isn't a > standard. It's just personal opinion vs personal opinion. If you were > going to do a lot of vboxvideo development, then it would be your > opinion which matters the most because you are doing the work. But > this is your first vboxvideo patch... > > Let's just leave it as-is. I agree with this comment. I'm just a newbie for this module and It isn't about checkpatch or CodingStyle. but I just wondered if my code has problem. regards, Sidong Yang > > regards, > dan carpenter > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel