[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 03/11] configure: check for sys/disk.h
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v9 03/11] configure: check for sys/disk.h |
Date: |
Tue, 26 Jan 2021 08:08:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 1/26/21 6:55 AM, Joelle van Dyne wrote:
> Previously, the only case where sys/disk.h does not exist is on
> platforms that define __DragonFly__. However, iOS also does not have
> this header. Previously, I had it as
>
> #if defined(__DragonFly__) || defined(CONFIG_IOS)
>
> But there was a code review comment that we should use feature flags
> instead of platform defines. So I added the HAS_SYS_DISK_H flag.
On technical lists, it's best to avoid top-posting, and to
instead reply inline to make the conversation easier to follow.
>
> -j
>
> On Mon, Jan 25, 2021 at 8:35 PM Warner Losh <imp@bsdimp.com> wrote:
>>
>>
>>
>> On Mon, Jan 25, 2021 at 6:33 PM Joelle van Dyne <j@getutm.app> wrote:
>>>
>>> Some BSD platforms do not have this header.
>>>
>>> Signed-off-by: Joelle van Dyne <j@getutm.app>
>>> ---
>>> meson.build | 1 +
>>> block.c | 2 +-
>>> block/file-posix.c | 2 +-
>>> 3 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 27110075df..6818d97df5 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -1117,6 +1117,7 @@ config_host_data.set('HAVE_PTY_H',
>>> cc.has_header('pty.h'))
>>> config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
>>> config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
>>> config_host_data.set('HAVE_HOST_BLOCK_DEVICE', have_host_block_device)
>>> +config_host_data.set('HAVE_SYS_DISK_H', cc.has_header('sys/disk.h'))
>>>
>>> ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
>>> arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST',
>>> 'CONFIG_BDRV_RO_WHITELIST']
>>> diff --git a/block.c b/block.c
>>> index 8b9d457546..c4cf391dea 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -54,7 +54,7 @@
>>> #ifdef CONFIG_BSD
>>> #include <sys/ioctl.h>
>>> #include <sys/queue.h>
>>> -#ifndef __DragonFly__
>>> +#if defined(HAVE_SYS_DISK_H)
>>> #include <sys/disk.h>
>>> #endif
>>> #endif
>>> diff --git a/block/file-posix.c b/block/file-posix.c
>>> index 11d2021346..666d3e7504 100644
>>> --- a/block/file-posix.c
>>> +++ b/block/file-posix.c
>>> @@ -2320,7 +2320,7 @@ again:
>>> }
>>> if (size == 0)
>>> #endif
>>> -#if defined(__APPLE__) && defined(__MACH__)
>>> +#if defined(HAVE_SYS_DISK_H) && defined(__APPLE__) && defined(__MACH__)
>>
>>
>> Why is this needed? __DragonFly__ doesn't define either __APPLE__ or __MACH__
Hmm we could also add:
config_host_data.set('HAVE_DKIOCGETBLOCKCOUNT', cc.compiles(...))
Then this block would be easier to read:
#if defined(HAVE_DKIOCGETBLOCKCOUNT)
...
(Maybe this is what Warner meant?)
>>
>> Warner
>>
>>>
>>> {
>>> uint64_t sectors = 0;
>>> uint32_t sector_size = 0;
>>> --
>>> 2.28.0
>>>
>>>
>