Hi Thomas, There is by far more than 2 issues, but these are the only ones that appeared to us.
The list of the issues that motivated the refactoring: 1. Only one error code for every fault (-EFAULT). 2. Copy-paste code from the `fget' function. 3. Ambiguous variable names. The `files' variable mean different things under the same block. This ruins readability of the code. 4. Overall placing of the all the code inside one `switch/case'. Modern kernel code style suggests placing these in the inline functions. 5. Usage of the copy-pasted `close_fd' function code. (I really should use `sys_close' here though). I can rewrite the refactoring in a set of small patches if this will aid reviewers. The dangerous bug that definitely must be fixed before the next release is the `struct file*' leakage. I can provide a simple patch for this as a separate submission and continue working on the refactoring patches. Is this plan OK for you? Pavel On Mon, Mar 23, 2015 at 1:15 PM, Thomas Monjalon <thomas.monjalon at 6wind.com> wrote: > Hi Pavel, > > You forgot the Signed-off-by line. > > Huawei, Changchun, any comment on this patch? > > 2015-03-18 15:16, Pavel Boldin: > > The `eventfd_link' module provides an API to "steal" fd from another > > process had been written with a bug that leaks `struct file' because > > of the extra reference counter increment and missing `fput' call. > > > > The other bug is using another process' `task_struct' without > incrementing > > a reference counter. > > As there are 2 bugs, you should provide 2 patches. > > > Fix these bugs and refactor the module. > > Why refactoring along with the bug fixes? > You'd have more chances to have your fixes integrated in 2.0 without > refactoring. > > Thanks > > > >
