Messages in this thread | | | Date | Fri, 08 Jan 2010 08:55:40 +0800 | From | Li Zefan <> | Subject | Re: [PATCH v5 1/4] cgroup: implement eventfd-based generic API for notifications |
| |
Kirill A. Shutemov wrote: > On Thu, Jan 7, 2010 at 3:01 AM, Paul Menage <menage@google.com> wrote: >> On Wed, Dec 30, 2009 at 7:57 AM, Kirill A. Shutemov >> <kirill@shutemov.name> wrote: >>> + >>> + if (!IS_ERR(efile)) >>> + fput(efile); >> While this is OK currently, it's a bit fragile. efile starts as NULL, >> and IS_ERR(NULL) is false. So if we jump to fail: before trying to do >> the eventfd_fget() then we'll try to fput(NULL), which will oops. This >> works because we don't currently jump to fail: until after >> eventfd_fget(), but someone could add an extra setup step between the >> kzalloc() and the eventfd_fget() which could fail. > > So we need to use IS_ERR_OR_NULL here instread of IS_ERR, don't we? >
Use multi labels is much better:
label4:: fput(cfile); label3: eventfd_ctx_put(event->eventfd); label2: fput(efile); label1: kfree(event);
compared to:
+fail: + if (!IS_ERR(cfile)) + fput(cfile); + + if (event && event->eventfd && !IS_ERR(event->eventfd)) + eventfd_ctx_put(event->eventfd); + + if (!IS_ERR(efile)) + fput(efile); + + kfree(event);
| |