Andy Shevchenko
2022-Sep-16 08:46 UTC
[PATCH] jailhouse: Hold reference returned from of_find_xxx API
On Fri, Sep 16, 2022 at 10:09 AM Liang He <windhl at 126.com> wrote:> At 2022-09-16 13:38:39, "Andy Shevchenko" <andy.shevchenko at gmail.com> wrote: > >On Fri, Sep 16, 2022 at 5:02 AM Liang He <windhl at 126.com> wrote: > >> At 2022-09-16 07:29:06, "Srivatsa S. Bhat" <srivatsa at csail.mit.edu> wrote: > >> >On 9/14/22 7:23 PM, Liang He wrote:...> >> >> static inline bool jailhouse_paravirt(void) > >> >> { > >> >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> >> + > >> >> + of_node_put(np); > >> >> + > >> >> + return np; > >> >> } > >> > > >> >Thank you for the fix, but returning a pointer from a function with a > >> >bool return type looks odd. Can we also fix that up please? > >> > >> Thanks for your review, how about following patch: > >> > >> - return of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + struct device_node *np = of_find_compatible_node(NULL, NULL, "jailhouse,cell"); > >> + > >> + of_node_put(np); > >> + > >> + return (np==NULL); > > >This will be opposite to the above. Perhaps you wanted > > Sorry, I wanted to use 'np!=NULL' > > > return !!np; > > > >Also possible (but why?) > > > > return np ? true : false; > > So, can I chose 'return np?true: false;' as the final patch?Of course you can, it's up to the maintainer(s) what to accept. -- With Best Regards, Andy Shevchenko