On Tue, 2018-03-27 at 21:13 +0100, Sitsofe Wheeler wrote: > I have a git tree (https://github.com/sitsofe/fio/commits/proc_group ) > that adds processor group support to fio CPU setting but to do so it > introduces a flag to target the build to Windows 7 and sets it to be > the default. If this change goes in it will mean if you would need to > explicitly set a flag to have an fio that will work for > WinXP/Vista/2003/2008 (before R2) etc. If you have time, any feedback > would be useful. > > If I don't hear any complaints I'll send a pull request to Jens in a day or two. Hello Sitsofe, My feedback about these patches is as follows: - Having to specify a target Windows version at build time seems wrong to me. What I did myself when I wrote Windows code 15 years ago was to detect at runtime which functionality is available and which functionality is not available. An initialization function could e.g. call LoadLibrary() and GetProcAddress(). The function pointer(s) returned by GetProcAddress() can be stored in global variables. Any function that needs code that is not available on every supported Windows version can then check these function pointers, starting with the most recently introduced function, and call a function that is available on the Windows platform fio has been started on. - Explicit XP / 2000 / etc. version checks seem unfortunate to me because I think these will lead to maintainability problems in the future. - Your patch series adds a lot of code in header files which will slow down the build. I think such code should be added to .c files instead. Thanks, Bart. ��.n��������+%������w��{.n�������^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
