[asterisk-dev] [Code Review] inotify(7) support for pbx_spool

[Mark Michelson]

> On 2009-02-12 18:24:45, Mark Michelson wrote:
> > /trunk/pbx/pbx_spool.c, line 499
> > <http://reviewboard.digium.com/r/137/diff/1/?file=2487#file2487line499>
> >
> >     As far as I can see, you never use this field of the buf struct.
> 
>  wrote:
>     Actually, I do.  It comes entirely from the inotify(7) subsystem, and it contains the pathname of the file whose change notification I am receiving.  If I omitted this element from the structure, then upon reading events from the inotify_fd, I would either get partial events, or I would overwrite a section of memory that wasn't a buffer.  When I read from the inotify_fd, I use buf, which flows through the iev structure and overflows into the name buffer (similar to our own ast_var, which has a name[0] element at the end of the struct).
>     
>     I've added a comment that explains this.

Ooh, that is clever :)


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/137/#review409
-----------------------------------------------------------


On 2009-02-12 19:06:24, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/137/
> -----------------------------------------------------------
> 
> (Updated 2009-02-12 19:06:24)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Instead of scanning the directory and trying to keep tabs on the mtime of the directory, an approach which has already been shown to have an issue with directory entry races, use the kernel builtin method of inotify(7) to identify every file as it gets created in the spool directory.
> 
> 
> Diffs
> -----
> 
>   /trunk/configure 175312 
>   /trunk/configure.ac 175312 
>   /trunk/include/asterisk/autoconfig.h.in 175312 
>   /trunk/pbx/pbx_spool.c 175312 
> 
> Diff: http://reviewboard.digium.com/r/137/diff
> 
> 
> Testing
> -------
> 
> Compile-time testing only.
> 
> 
> Thanks,
> 
> Tilghman
> 
>