On 02/18/2011 02:59 AM, Markus Armbruster wrote:
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.
Okay, make it a shorter name then DEFPROP_UINT32. Regards, Anthony Liguori
