Re: [libvirt] [PATCH 0/6] Introduce VIR_AUTOCLOSE macro for closing fd automatically

On Tue, Sep 11, 2018 at 11:49:11PM +0800, Shi Lei wrote: > On 2018-09-11 at 20:50, Erik Skultety wrote: > >On Mon, Sep 10, 2018 at 11:47:49AM +0800, Shi Lei wrote: > >> This series introduce VIR_AUTOCLOSE macro which can force close fd of the file > >> automatically when the fd goes out of scope. It's used to eliminate VIR_FORCE_CLOSE > >> in cleanup sections. The new macro consults VIR_AUTOFREE and VIR_AUTOPTR. > >> > >> Shi Lei (6): > >>   util: file: introduce VIR_AUTOCLOSE macro to close fd of the file > >>     automatically > >>   util: file: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE in cleanup > >>     sections > >>   util: netdevbridge: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE in > >>     cleanup sections > >>   util: netdev: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE in cleanup > >>     sections > >>   phyp: driver: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE in cleanup > >>     sections > >>   uml: conf: use VIR_AUTOCLOSE instead of VIR_FORCE_CLOSE in cleanup > >>     sections > >> > >>  src/phyp/phyp_driver.c     |  29 ++--- > >>  src/uml/uml_conf.c         |  13 +- > >>  src/util/virfile.c         |  21 +-- > >>  src/util/virfile.h         |  20 ++- > >>  src/util/virnetdev.c       | 253 +++++++++++++------------------------ > >>  src/util/virnetdevbridge.c | 120 ++++++------------ > > > >This touches only a handful of util modules and random drivers, there should be > >some consistency here, so if we're introducing something like this, then we > >should do it in a similar manner as we did for VIR_AUTO{FREE,PTR}, i.e. util/ > >first (btw. we still haven't even finished that one yet). > > > >Erik > > Okay. But how to do in the v2 series. > > Whether v2 should only deal with files in the util/  ? Yeah, I think that's the best approach we can take, since we still haven't finished converting util to the other macros, we should start with the those modules within util/ that are already completely switched to AUTO{FREE/PTR} and similarly do it in batches, as reviewing 30+ patches on one go is always demanding... Thanks, Erik