| Message ID | 1342433706-3376-1-git-send-email-jing.huang.pku@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 16 July 2012 11:15, Jing Huang <jing.huang.pku@gmail.com> wrote: > This patch excludes SO_TIMESTAMP cmsg_type from unsuppoted ancillary data. "unsupported". > > Signed-off-by: Jing Huang <jing.huang.pku@gmail.com> > --- > linux-user/syscall.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index fc8690d..9fce57c 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -1349,7 +1349,9 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, > target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type); > target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len)); > > - if (cmsg->cmsg_level != TARGET_SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) { > + if (cmsg->cmsg_level != TARGET_SOL_SOCKET || > + (cmsg->cmsg_type != SCM_RIGHTS && > + cmsg->cmsg_type != SO_TIMESTAMP)) { > gemu_log("Unsupported ancillary data: %d/%d\n", cmsg->cmsg_level, cmsg->cmsg_type); > memcpy(target_data, data, len); > } else { This is kind of ugly -- it causes us to process a SO_TIMESTAMP payload (which is a 'struct timeval') in the same way as an SCM_RIGHTS payload (which is an array of file descriptors). This happens to work because a struct timeval is a pair of 32 bit integers, and so "tswap32() all the words in the data" works, but it looks pretty weird. If you want to do it this way you need at least an explanatory comment and really the variables 'fd', 'target_fd', 'numfds' would need to change to something more generic and less fd-specific. -- PMM
On Mon, Jul 16, 2012 at 2:45 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 16 July 2012 11:15, Jing Huang <jing.huang.pku@gmail.com> wrote: >> This patch excludes SO_TIMESTAMP cmsg_type from unsuppoted ancillary data. > > "unsupported". > >> >> Signed-off-by: Jing Huang <jing.huang.pku@gmail.com> >> --- >> linux-user/syscall.c | 4 +++- >> 1 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index fc8690d..9fce57c 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -1349,7 +1349,9 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, >> target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type); >> target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len)); >> >> - if (cmsg->cmsg_level != TARGET_SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) { >> + if (cmsg->cmsg_level != TARGET_SOL_SOCKET || >> + (cmsg->cmsg_type != SCM_RIGHTS && >> + cmsg->cmsg_type != SO_TIMESTAMP)) { >> gemu_log("Unsupported ancillary data: %d/%d\n", cmsg->cmsg_level, cmsg->cmsg_type); >> memcpy(target_data, data, len); >> } else { > > This is kind of ugly -- it causes us to process a SO_TIMESTAMP > payload (which is a 'struct timeval') in the same way as an > SCM_RIGHTS payload (which is an array of file descriptors). > This happens to work because a struct timeval is a pair of > 32 bit integers, and so "tswap32() all the words in the data" > works, but it looks pretty weird. If you want to do it this > way you need at least an explanatory comment and really the > variables 'fd', 'target_fd', 'numfds' would need to change > to something more generic and less fd-specific. > > -- PMM Hmm.. I give up this patch. It is ugly indeed. Additionally, qemu_log doesn't disturb the normal execution of linux-user and show error message either.
On 19 July 2012 15:33, Jing Huang <jing.huang.pku@gmail.com> wrote: > On Mon, Jul 16, 2012 at 2:45 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 16 July 2012 11:15, Jing Huang <jing.huang.pku@gmail.com> wrote: >>> This patch excludes SO_TIMESTAMP cmsg_type from unsuppoted ancillary data. >> >> "unsupported". >> >>> >>> Signed-off-by: Jing Huang <jing.huang.pku@gmail.com> >>> --- >>> linux-user/syscall.c | 4 +++- >>> 1 files changed, 3 insertions(+), 1 deletions(-) >>> >>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >>> index fc8690d..9fce57c 100644 >>> --- a/linux-user/syscall.c >>> +++ b/linux-user/syscall.c >>> @@ -1349,7 +1349,9 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, >>> target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type); >>> target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len)); >>> >>> - if (cmsg->cmsg_level != TARGET_SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) { >>> + if (cmsg->cmsg_level != TARGET_SOL_SOCKET || >>> + (cmsg->cmsg_type != SCM_RIGHTS && >>> + cmsg->cmsg_type != SO_TIMESTAMP)) { >>> gemu_log("Unsupported ancillary data: %d/%d\n", cmsg->cmsg_level, cmsg->cmsg_type); >>> memcpy(target_data, data, len); >>> } else { >> >> This is kind of ugly -- it causes us to process a SO_TIMESTAMP >> payload (which is a 'struct timeval') in the same way as an >> SCM_RIGHTS payload (which is an array of file descriptors). >> This happens to work because a struct timeval is a pair of >> 32 bit integers, and so "tswap32() all the words in the data" >> works, but it looks pretty weird. If you want to do it this >> way you need at least an explanatory comment and really the >> variables 'fd', 'target_fd', 'numfds' would need to change >> to something more generic and less fd-specific. >> >> -- PMM > > > Hmm.. I give up this patch. It is ugly indeed. Additionally, qemu_log > doesn't disturb the normal execution of linux-user and show error > message either. I think it's salvageable. As I say, you can either genericise the "copy a pile of 32 bit values" code, or just have another clause to the if() specifically for handling struct timeval payload data. -- PMM
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index fc8690d..9fce57c 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1349,7 +1349,9 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type); target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len)); - if (cmsg->cmsg_level != TARGET_SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) { + if (cmsg->cmsg_level != TARGET_SOL_SOCKET || + (cmsg->cmsg_type != SCM_RIGHTS && + cmsg->cmsg_type != SO_TIMESTAMP)) { gemu_log("Unsupported ancillary data: %d/%d\n", cmsg->cmsg_level, cmsg->cmsg_type); memcpy(target_data, data, len); } else {
This patch excludes SO_TIMESTAMP cmsg_type from unsuppoted ancillary data. Signed-off-by: Jing Huang <jing.huang.pku@gmail.com> --- linux-user/syscall.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)