Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Commit

Permalink
linux: try inotify_init if inotify_init1 returns ENOSYS
Browse files Browse the repository at this point in the history 
The kernel may be older than the kernel headers that libuv is compiled against.
  • Loading branch information
@bnoordhuis
bnoordhuis committed Mar 15, 2012
1 parent 6dcce92 commit 66a959c
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/unix/linux.c
View file
Expand Up @@ -170,11 +170,16 @@ uint64_t uv_get_total_memory(void) {
#if HAVE_INOTIFY_INIT || HAVE_INOTIFY_INIT1

static int new_inotify_fd(void) {
#if HAVE_INOTIFY_INIT1
return inotify_init1(IN_NONBLOCK | IN_CLOEXEC);
#else
int fd;

#if HAVE_INOTIFY_INIT1
fd = inotify_init1(IN_NONBLOCK | IN_CLOEXEC);
if (fd != -1)
return fd;
if (errno != ENOSYS)
return -1;
#endif

if ((fd = inotify_init()) == -1)
return -1;

Expand All @@ -184,7 +189,6 @@ static int new_inotify_fd(void) {
}

return fd;
#endif
}


Expand Down

4 comments on commit 66a959c

@ttback
Copy link

@ttback ttback commented on 66a959c Mar 15, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Ben, wouldn't your logic be expressed a little simpler if we refactor the code into following:

static int new_inotify_fd(void) {
   int fd;

#if HAVE_INOTIFY_INIT1
  fd = inotify_init1(IN_NONBLOCK | IN_CLOEXEC);  //fd = inotify_init1

  if (fd != -1)  //if we have inotify_init1, return inotify_init1()
    return fd;                                 


   if (errno == ENOSYS | (fd = inotify_init()) != -1 ){   //either return -1 | inotify_init()
     return fd;
   }
#endif

   return -1;   //Otherwise, always return -1 if none of the above conditions are satisfied 
-#endif
}

@bnoordhuis
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what you feel is not simple about the current approach?

By the way, there's a bug in your patch's logic: it fails to set the FD_CLOEXEC flag if inotify_init1() returned ENOSYS. errno is only set on error, syscalls that succeed don't touch it.

It also retries if inotify_init1() fails with e.g. EMFILE or ENOMEM. Masking errors like that is generally considered an undesirable trait.

@ttback
Copy link

@ttback ttback commented on 66a959c Mar 16, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure of the reason why the code goes from return fd to return -1 two times and then have to return fd at the end.

Not sure what the meaning of static int new_inotify_fd(void) {} before the final return as well. Could you tell me more?

@bnoordhuis
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure of the reason why the code goes from return fd to return -1 two times and then have to return fd at the end.

return fd instead of return -1 would work too because fd is -1 is at that point but return -1 takes less brain cycles to process because you don't have to look up (or infer) the value of fd in your mental database of variables.

Not sure what the meaning of static int new_inotify_fd(void) {} before the final return as well. Could you tell me more?

It's a patch marker, something that git inserts. It's not actual code.

Please sign in to comment.