Re: sosetopt(): merge SO_SND* with corresponding SO_RCV* cases
On Wed, Aug 09, 2023 at 12:41:18AM +0300, Vitaliy Makkoveev wrote:
> I think it's better to merge SO_BINDANY cases from both switch blocks.
> This time SO_LINGER case is separated, so there is no reason for
> dedicated switch block.
OK bluhm@
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.307
> diff -u -p -r1.307 uipc_socket.c
> --- sys/kern/uipc_socket.c3 Aug 2023 09:49:08 - 1.307
> +++ sys/kern/uipc_socket.c8 Aug 2023 21:35:50 -
> @@ -1800,13 +1800,6 @@ sosetopt(struct socket *so, int level, i
> error = ENOPROTOOPT;
> } else {
> switch (optname) {
> - case SO_BINDANY:
> - if ((error = suser(curproc)) != 0) /* XXX */
> - return (error);
> - break;
> - }
> -
> - switch (optname) {
>
> case SO_LINGER:
> if (m == NULL || m->m_len != sizeof (struct linger) ||
> @@ -1824,6 +1817,9 @@ sosetopt(struct socket *so, int level, i
>
> break;
Can you add a newline here? All other cases have one.
> case SO_BINDANY:
> + if ((error = suser(curproc)) != 0) /* XXX */
> + return (error);
> + /* FALLTHROUGH */
> case SO_DEBUG:
> case SO_KEEPALIVE:
> case SO_USELOOPBACK:
Re: sosetopt(): merge SO_SND* with corresponding SO_RCV* cases
On Tue, Aug 08, 2023 at 10:40:46PM +0200, Alexander Bluhm wrote:
> On Fri, Aug 04, 2023 at 12:38:23AM +0300, Vitaliy Makkoveev wrote:
> > @@ -1856,6 +1856,9 @@ sosetopt(struct socket *so, int level, i
> > case SO_SNDLOWAT:
> > case SO_RCVLOWAT:
> > {
> > + struct sockbuf *sb = (optname == SO_SNDBUF ||
> > + optname == SO_SNDLOWAT ?
> > + &so->so_snd : &so->so_rcv);
> > @@ -1910,6 +1896,8 @@ sosetopt(struct socket *so, int level, i
> > case SO_SNDTIMEO:
> > case SO_RCVTIMEO:
> > {
> > + struct sockbuf *sb = (optname == SO_SNDTIMEO ?
> > + &so->so_snd : &so->so_rcv);
>
> Would it be nicer to set sb in the switch (optname) at the begining?
>
> struct sockbuf *sb = NULL;
>
> switch (optname) {
> case SO_SNDBUF:
> case SO_SNDLOWAT:
> case SO_SNDTIMEO:
> sb = &so->so_snd;
> break;
> case SO_RCVBUF:
> case SO_RCVLOWAT:
> case SO_RCVTIMEO:
> sb = &so->so_rcv;
> break;
> case SO_BINDANY:
> ...
> }
>
> Anyway, OK bluhm@
>
I think it's better to merge SO_BINDANY cases from both switch blocks.
This time SO_LINGER case is separated, so there is no reason for
dedicated switch block.
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.307
diff -u -p -r1.307 uipc_socket.c
--- sys/kern/uipc_socket.c 3 Aug 2023 09:49:08 - 1.307
+++ sys/kern/uipc_socket.c 8 Aug 2023 21:35:50 -
@@ -1800,13 +1800,6 @@ sosetopt(struct socket *so, int level, i
error = ENOPROTOOPT;
} else {
switch (optname) {
- case SO_BINDANY:
- if ((error = suser(curproc)) != 0) /* XXX */
- return (error);
- break;
- }
-
- switch (optname) {
case SO_LINGER:
if (m == NULL || m->m_len != sizeof (struct linger) ||
@@ -1824,6 +1817,9 @@ sosetopt(struct socket *so, int level, i
break;
case SO_BINDANY:
+ if ((error = suser(curproc)) != 0) /* XXX */
+ return (error);
+ /* FALLTHROUGH */
case SO_DEBUG:
case SO_KEEPALIVE:
case SO_USELOOPBACK:
Re: sosetopt(): merge SO_SND* with corresponding SO_RCV* cases
On Fri, Aug 04, 2023 at 12:38:23AM +0300, Vitaliy Makkoveev wrote:
> @@ -1856,6 +1856,9 @@ sosetopt(struct socket *so, int level, i
> case SO_SNDLOWAT:
> case SO_RCVLOWAT:
> {
> + struct sockbuf *sb = (optname == SO_SNDBUF ||
> + optname == SO_SNDLOWAT ?
> + &so->so_snd : &so->so_rcv);
> @@ -1910,6 +1896,8 @@ sosetopt(struct socket *so, int level, i
> case SO_SNDTIMEO:
> case SO_RCVTIMEO:
> {
> + struct sockbuf *sb = (optname == SO_SNDTIMEO ?
> + &so->so_snd : &so->so_rcv);
Would it be nicer to set sb in the switch (optname) at the begining?
struct sockbuf *sb = NULL;
switch (optname) {
case SO_SNDBUF:
case SO_SNDLOWAT:
case SO_SNDTIMEO:
sb = &so->so_snd;
break;
case SO_RCVBUF:
case SO_RCVLOWAT:
case SO_RCVTIMEO:
sb = &so->so_rcv;
break;
case SO_BINDANY:
...
}
Anyway, OK bluhm@
sosetopt(): merge SO_SND* with corresponding SO_RCV* cases
The only difference is the socket buffer.
As bonus, in the future solock() will be easily replaced by sblock()
instead pushing it down to each SO_SND* and SO_RCV* case.
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.307
diff -u -p -r1.307 uipc_socket.c
--- sys/kern/uipc_socket.c 3 Aug 2023 09:49:08 - 1.307
+++ sys/kern/uipc_socket.c 3 Aug 2023 21:20:58 -
@@ -1856,6 +1856,9 @@ sosetopt(struct socket *so, int level, i
case SO_SNDLOWAT:
case SO_RCVLOWAT:
{
+ struct sockbuf *sb = (optname == SO_SNDBUF ||
+ optname == SO_SNDLOWAT ?
+ &so->so_snd : &so->so_rcv);
u_long cnt;
if (m == NULL || m->m_len < sizeof (int))
@@ -1867,40 +1870,23 @@ sosetopt(struct socket *so, int level, i
solock(so);
switch (optname) {
case SO_SNDBUF:
- if (so->so_snd.sb_state & SS_CANTSENDMORE) {
- error = EINVAL;
- break;
- }
- if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
- sbreserve(so, &so->so_snd, cnt)) {
- error = ENOBUFS;
- break;
- }
- so->so_snd.sb_wat = cnt;
- break;
-
case SO_RCVBUF:
- if (so->so_rcv.sb_state & SS_CANTRCVMORE) {
+ if (sb->sb_state &
+ (SS_CANTSENDMORE | SS_CANTRCVMORE)) {
error = EINVAL;
break;
}
- if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
- sbreserve(so, &so->so_rcv, cnt)) {
+ if (sbcheckreserve(cnt, sb->sb_wat) ||
+ sbreserve(so, sb, cnt)) {
error = ENOBUFS;
break;
}
- so->so_rcv.sb_wat = cnt;
+ sb->sb_wat = cnt;
break;
-
case SO_SNDLOWAT:
- so->so_snd.sb_lowat =
- (cnt > so->so_snd.sb_hiwat) ?
- so->so_snd.sb_hiwat : cnt;
- break;
case SO_RCVLOWAT:
- so->so_rcv.sb_lowat =
- (cnt > so->so_rcv.sb_hiwat) ?
- so->so_rcv.sb_hiwat : cnt;
+ sb->sb_lowat = (cnt > sb->sb_hiwat) ?
+ sb->sb_hiwat : cnt;
break;
}
sounlock(so);
@@ -1910,6 +1896,8 @@ sosetopt(struct socket *so, int level, i
case SO_SNDTIMEO:
case SO_RCVTIMEO:
{
+ struct sockbuf *sb = (optname == SO_SNDTIMEO ?
+ &so->so_snd : &so->so_rcv);
struct timeval tv;
uint64_t nsecs;
@@ -1925,14 +1913,7 @@ sosetopt(struct socket *so, int level, i
nsecs = INFSLP;
solock(so);
- switch (optname) {
- case SO_SNDTIMEO:
- so->so_snd.sb_timeo_nsecs = nsecs;
- break;
- case SO_RCVTIMEO:
- so->so_rcv.sb_timeo_nsecs = nsecs;
- break;
- }
+ sb->sb_timeo_nsecs = nsecs;
sounlock(so);
break;
}
