Re: [PATCH v3 0/8] refactor the filter process code into a reusable module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ben Peart <peartben@xxxxxxxxx> writes:

> Ben Peart (8):
>   pkt-line: add packet_writel() and packet_read_line_gently()
>   convert: Update convert to use new packet_writel() function
>   convert: Split start_multi_file_filter into two separate functions
>   convert: Separate generic structures and variables from the filter
>     specific ones
>   convert: Update generic functions to only use generic data structures
>   convert: rename reusable sub-process functions
>   sub-process: move sub-process functions into separate files
>   convert: Update subprocess_read_status to not die on EOF

This presentation is much easier to digest, compared to the large
ball of wax we saw previously.  It highlights the key modification
that cmd2process is now "subclassed" from subprocess_entry which is
a more generic structure by embedding the latter at the beginning,
and have its user start_multi_file_filter_fn() explicitly downcast
the latter to the former around patches 4/8 and 5/8.

If I were doing this series, I would organize the first two slightly
differently, namely:

 * 1/8 just adds packet_read_line_gently().

 * 2/8 moves packet_write_line() from convert.c to pkt-line.c while
   renaming it, with the justification that this function must be
   made more widely available.  It would naturally involves
   adjusting existing callers.

because write and read done in your 1/8 are independent and
orthogonal changes, and doing it that way also avoids needless
temporary duplication of the same function.

I may later have further comments on 3-8/8 after giving them another
read, but I haven't seen anything questionable in them so far.

Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]