[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] Recommendations for adding log statements after checking
From: |
Darshit Shah |
Subject: |
Re: [Bug-wget] Recommendations for adding log statements after checking setsockopt() |
Date: |
Mon, 15 Oct 2018 13:14:01 +0200 |
User-agent: |
NeoMutt/20180716 |
Hi,
Thanks for the analysis! However, the code that you have identified comes from
gnulib, which is essentially a statically linked library. As a result code in
there is out of bounds for us to change.
Also, if you see closely, the function you've pointed to, is the
"rpl_setsockopt", that is, it is a replacement function for systems where
setsockopt doesn't behave in a sane manner. Hence, all the invokations to
setsockopt are indeed checked and logged.
* niuxu <address@hidden> [181015 11:56]:
> Our team works on enhance logging practices by learning from historical log
> revisions in evolution.
> We find that 2 patches have added validation code about the return value of
> setsockopt() along with logging statements.
>
>
> So we suggest that the return value of setsockopt() should be checked and
> logged if the check pass.
>
> And, we find 1 missed spot in line 35 of wget-1.19.2/lib/setsockopt.c:
> int
> rpl_setsockopt (int fd, int level, int optname, const void *optval, socklen_t
> optlen)
> {
> ...
> if (level == SOL_SOCKET
> && (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
> {
> const struct timeval *tv = optval;
> int milliseconds = tv->tv_sec * 1000 + tv->tv_usec / 1000;
> optval = &milliseconds;
> r = setsockopt (sock, level, optname, optval, sizeof (int));
> }
> else
> {
> r = setsockopt (sock, level, optname, optval, optlen);
> }
> if (r < 0)
> set_winsock_errno ();
>
> return r;
> }
>
> And the 2 patches that support us are:
> 1) In line 334 of File: wget-1.18/src/connect.c
> if (opt.limit_rate && opt.limit_rate < 8192)
> {
> int bufsize = opt.limit_rate;
> if (bufsize < 512)
> bufsize = 512; /* avoid pathologically small values */
> #ifdef SO_RCVBUF
> - setsockopt (sock, SOL_SOCKET, SO_RCVBUF,
> - (void *)&bufsize, (socklen_t)sizeof (bufsize));
> + if (setsockopt (sock, SOL_SOCKET, SO_RCVBUF,
> + (void *) &bufsize, (socklen_t) sizeof (bufsize)))
> + logprintf (LOG_NOTQUIET, _("setsockopt SO_RCVBUF failed: %s\n"),
> + strerror (errno));
> #endif
>
> 2) In line 474 of File: wget-1.18/src/connect.c
> sock = socket (bind_address->family, SOCK_STREAM, 0);
> if (sock < 0)
> return -1;
>
> #ifdef SO_REUSEADDR
> - setsockopt (sock, SOL_SOCKET, SO_REUSEADDR, setopt_ptr, setopt_size);
> + if (setsockopt (sock, SOL_SOCKET, SO_REUSEADDR, setopt_ptr, setopt_size))
> + logprintf (LOG_NOTQUIET, _("setsockopt SO_REUSEADDR failed: %s\n"),
> + strerror (errno));
> #endif
>
> Thanks for your reading and we are looking forward to your reply about the
> correctness of our suggestion.
> May you a good day! ^^
>
> Best Regards,
> Xu
--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6
signature.asc
Description: PGP signature