Anthony Liguori <anth...@codemonkey.ws> writes: > On 02/17/2011 07:06 AM, Amit Shah wrote: >> On (Tue) 15 Feb 2011 [10:43:42], Anthony Liguori wrote: >> >> >>>> #define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ >>>> - DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \ >>>> + DEFINE_PROP_DRIVE("drive", _state, _conf.bs, ""), \ >>>> DEFINE_PROP_UINT16("logical_block_size", _state, \ >>>> - _conf.logical_block_size, 512), \ >>>> + _conf.logical_block_size, 512, ""), \ >>>> DEFINE_PROP_UINT16("physical_block_size", _state, \ >>>> - _conf.physical_block_size, 512), \ >>>> - DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ >>>> - DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ >>>> - DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ >>>> + _conf.physical_block_size, 512, ""), \ >>>> + DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0, ""), \ >>>> + DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0, ""), \ >>>> + DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1, ""), \ >>>> DEFINE_PROP_UINT32("discard_granularity", _state, \ >>>> - _conf.discard_granularity, 0) >>>> + _conf.discard_granularity, 0, "") >>>> >>> This is pretty horribly ugly. If we were going this, we should at >>> least introduce new defines that include a documentation field and >>> not just add empty documentation fields. >>> >> We've discussed this in the past. Once this patch series gets in, >> I'll work to fill in the documentation here along with the >> maintainers. >> > > It means you're touching everything twice instead of touching > everything once. That's unnecessary churn and blame breakage. > > It's still just as greppable if you use a new name.
What names would you suggest? DEFINE_PROP_<FOO>_WITH_DOCS()? For all fifteen <FOO>s? Not a good idea, because the longer name makes doing the right thing even less attractive. We better encourage doing the right thing.