Re: [Qemu-devel] [PATCH] Fixes a bug that tries to use the unimplemented function fdatasync on Mac OS X.

[Pierre Riteau]

On 8 déc. 2009, at 19:35, Blue Swirl wrote:

> On Tue, Dec 8, 2009 at 4:35 PM, Pierre Riteau <address@hidden> wrote:
>> On 1 oct. 2009, at 18:13, Blue Swirl wrote:
>> 
>>> On Thu, Oct 1, 2009 at 6:30 PM, G 3 <address@hidden> wrote:
>>>> On Sep 30, 2009, at 3:16 PM, Anthony Liguori wrote:
>>>> 
>>>> G 3 wrote:
>>>> 
>>>> This patch fixes a problem in the file cutils.c that prevents qemu from
>>>> being built on Mac OS X. This patch makes sure fsync is used instead.
>>>> 
>>>> What is this against?  It doesn't apply to git.
>>>> 
>>>> This is against the savannah
>>>> repository: git://git.savannah.nongnu.org/qemu.git. Is this the wrong
>>>> repository?
>>> 
>>> The patch should not be needed since
>>> 5f6b9e8fd5b9516170e582d9b6c27c98519a8031. Do you still have a problem?
>> 
>> Sorry to dig up this old thread, but I just tried compiling qemu on my Mac 
>> and I see a warning when it compiles:
>> cutils.c: In function ‘qemu_fdatasync’:
>> cutils.c:128: warning: implicit declaration of function ‘fdatasync’
>> 
>> I'm running OS X 10.6.2 with Xcode 3.2.1.
>> 
>> Why this behavior? Because the configure check introduced in 5f6b9e finds 
>> that fdatasync is available.
>> Although the fdatasync function is not referenced in the standard includes 
>> (hence the warning), it is present as a syscall:
>> /usr/include/sys/syscall.h:#define      SYS_fdatasync      187
>> Moreover, a fdatasync symbol is available through libSystem (which is why 
>> the linking done by the configure check works).
>> 
>> It's not clear what is the best solution here. Googling the issue, some 
>> projects add the fdatasync prototype to their headers to avoid warnings.
>> Other assume that fdatasync could be a noop and fall back to fsync.
>> 
>> I would go with the second second: the function is not defined, there is no 
>> man page, let's assume it doesn't work.
>> To fix the configure check we can simply add -Werror to the compile_prog 
>> cflags (I will send a patch if you agree with this).
> 
> I'd rather add something like
> #ifdef __APPLE__
> exit(1);
> #endif
> (or the shell equivalent for configure) and a comment with your explanation.


I'm fine with that too.

-- 
Pierre Riteau -- http://perso.univ-rennes1.fr/pierre.riteau/