All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <chengang@emindsoft.com.cn>
To: Laurent Vivier <laurent@vivier.eu>, riku.voipio@iki.fi
Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt
Date: Fri, 8 Jan 2016 17:40:34 +0800	[thread overview]
Message-ID: <568F8412.4000507@emindsoft.com.cn> (raw)
In-Reply-To: <568F7270.5000506@vivier.eu>


Firstly, thank you very much for your careful work.

On 2016年01月08日 16:25, Laurent Vivier wrote:
> 
> 
> Le 08/01/2016 02:59, chengang@emindsoft.com.cn a écrit :
[...]
>> @@ -1692,10 +1693,30 @@ static abi_long do_getsockopt(int sockfd, int level, int optname,
>>          switch (optname) {
>>          /* These don't just return a single integer */
>>          case TARGET_SO_LINGER:
>> -        case TARGET_SO_RCVTIMEO:
>> -        case TARGET_SO_SNDTIMEO:
>>          case TARGET_SO_PEERNAME:
>>              goto unimplemented;
>> +
> 
> useless blank line
> 

OK.

>> +        case TARGET_SO_RCVTIMEO:
>> +            optname = SO_RCVTIMEO;
>> +            goto time_case;
>> +        case TARGET_SO_SNDTIMEO:
>> +            optname = SO_SNDTIMEO;
>> +        time_case:
> 
> Something like in "int_case", I think optlen is a pointer, not the length:
> 
>         if (get_user_u32(len, optlen))
>             return -TARGET_EFAULT;
>         if (len < 0)
>             return -TARGET_EINVAL;
> 

OK.

> 
>> +            if (optlen < sizeof(struct target_timeval)) {
>> +                return -TARGET_EINVAL;
>> +            }
> 
> You don't have to check the len (kernel doesn't), EINVAL is not listed
> in the getsockopt() error cases, it should be an EFAULT, and this will
> be managed by copy_to_user_timeval().
> 

OK.

>> +            lv = sizeof(tv);
>> +            ret = get_errno(getsockopt(sockfd, level, optname, &tv, &lv));
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
> 
>  if (len > lv)
>     len = lv;
> 

OK.

>> +            if (copy_to_user_timeval(optval_addr, &tv)) {
>> +                return -TARGET_EFAULT;
>> +            }
>> +            if (put_user_u32(sizeof(struct target_timeval), optlen)) {
>> +                return -TARGET_EFAULT;
>> +            }
> 
>         if (put_user_u32(len, optlen))
>             return -TARGET_EFAULT;
> 

OK. I shall send patch v2 for it, in the next week.


Thanks.
-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

  reply	other threads:[~2016-01-08  9:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-08  1:59 [Qemu-devel] [PATCH] linux-user/syscall.c: Add SO_RCVTIMEO and SO_SNDTIMEO for getsockopt chengang
2016-01-08  8:25 ` Laurent Vivier
2016-01-08  9:40   ` Chen Gang [this message]
2016-01-11  6:31     ` Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=568F8412.4000507@emindsoft.com.cn \
    --to=chengang@emindsoft.com.cn \
    --cc=laurent@vivier.eu \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=riku.voipio@iki.fi \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.