2021-06-09 18:52 (UTC+0300), Dmitry Kozlyuk: > 2021-05-20 20:17 (UTC+0000), Akhil Goyal: > > > > > > 2021-05-20 18:59 (UTC+0000), Akhil Goyal: > > > > > Windows system headers define `s_addr`, `min`, and `max` macros which > > > > > break structure definitions containing fields with one of these names. > > > > > Undefining those macros would break consumer code that relies on > > > them. > > > > > > > > > > > > > From the commit message the requirement for changing the structure > > > definitions > > > > Is not clear. Please note that 'min' - 'max' are not macros. These are > > > > > > > variables of a > > > > structure which should not break any other structure/Macro in windows. > > > > > > > > > > Err, yes, that's what the commit message says. > > > Structure fields of course break nothing; they are broken by Windows > > > macros. > > > Would this make more sense? > > > > > > > > > Windows headers define `s_addr`, `min`, and `max` as macros. > > > If DPDK headers are included after Windows ones, DPDK structure > > > definitions containing fields with these names get broken. > > > If DPDK headers undefined these macros, it could break consumer > > > code > > > relying on these macros. It is proposed to rename structure fields > > > in DPDK, because Win32 headers are more widely used and harder > > > to fix. > > > > Yes it makes more sense now. But ideally it should be fixed in windows. > > This may be just one such issue, there may be many more. > > Will this also mean that nobody can define a local variable 'min'? > > Is this acceptable? > > Only in public headers. There happens to be one such, rte_lru_x86.h. > > > Any macro definition in a subsystem should have a prefix to denote that, > > Just like in DPDK 'RTE_' is added. > > Macros with generic names should be avoided so that we do not get into > > these issues. > > > > Adding more people for comments. I don't have a good feeling about > > this change. > > Friendly ping to everyone Akhil cc'ed. > As far as I understand, if we want to fix it in 21.11, > deprecation notice should make it into 21.08.
Friendly ping v2. Hopefully Tyler's answer will help with the decision.
