|
From: | Laurent Vivier |
Subject: | Re: [Qemu-devel] [PATCH] linux-user: Allow getdents to be provided by getdents64 |
Date: | Mon, 3 Jun 2013 13:50:01 +0200 (CEST) |
I agree with all your comments.
Tested-by: Laurent Vivier <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
> Le 3 juin 2013 à 13:28, Peter Maydell <address@hidden> a écrit : > > > On 3 June 2013 12:11, Laurent Vivier <address@hidden> wrote: > > Tested on m68k on x86_64 as described in the patch comment, in a a > > debian-etch linux container. > > > > Works fine, except the drec_len differs between getdents() and getdents64(). > > >> Le 2 juin 2013 à 19:10, Peter Maydell <address@hidden> a écrit : > >> + namelen = strlen(de->d_name); > >> + treclen = offsetof(struct target_dirent, d_name) + namelen + 2; > >> + treclen = QEMU_ALIGN_UP(treclen, sizeof(abi_long)); > > > > You should use ABI_LONG_ALIGNMENT instead of sizeof(abi_long), as some > > targets (at least m68k) don't align on long size. > > I'm following what the kernel source does: > http://lxr.linux.no/#linux+v3.5/fs/readdir.c#L154 > > which if I'm not misreading it also aligns based on > the size of the type, not its alignment requirements. > (the C spec guarantees that the former is never less strict > than the latter, so it's just unnecessary padding, not > misaligning the next record). > > Looking at our code for 32-bit-guest-on-64-bit-host > getdents-in-terms-of-getdents, I think it is actually > the existing code which is not getting the record > length correct, which is why you're seeing a difference > between that and the code added in this patch. If you > have access to the real hardware (or even just a system > emulated QEMU I guess) I would expect to see that the > record lengths it uses match this patch's results. > > getdents64() calculates the record length by aligning to > a multiple of sizeof(u64), and getdents() does it by > aligning to a multiple of sizeof(long). So if you work > out the record length for target_dirent by back-calculating > it from that of linux_dirent when the sizeof(long) > is different between host and target, you'll end up > with possibly a word of extra padding you didn't need. > This won't hurt any guest code, since it shouldn't > care, but it is why I chose in my code to determine the > length of the filename via strlen(), so I could get > the record length exactly correct. > > (The returned value, ie total number of bytes read > into the buffer, will unavoidably differ, because > the conversion process packs the results down into > a smaller space. This shouldn't break guests either, > fortunately.) > > thanks > -- PMM |
[Prev in Thread] | Current Thread | [Next in Thread] |