On Wed, Feb 15, 2023 at 08:08:42PM +0100, Alexander Bluhm wrote:
> On Mon, Jan 30, 2023 at 05:03:30PM +0300, Vitaliy Makkoveev wrote:
> > It makes sense to push solock() down to sosetopt() too. For a half cases
> > (*pr_ctloutput)() is just null op, so there is no reason to call it.
> > Also, a lot of things could be done without solock() held.
>
> You do a bunch of things together
> - push solock()
> - move code around
> - return error instead of error =
> - set sb to &so->so_snd or &so->so_rcv
>
> Some of the refactorings make sense, others change the code just
> to be different in my eyes.
>
> Can we discuss theses things in separate diffs?
>
Does this "set sb to &so->so_snd or &so->so_rcv" chunk look reasonable?
Merge sosetopt() SO_SND* with corresponding SO_RCV* code paths. The
difference they have is only the socket buffer.
Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.301
diff -u -p -r1.301 uipc_socket.c
--- sys/kern/uipc_socket.c 10 Feb 2023 14:34:17 -0000 1.301
+++ sys/kern/uipc_socket.c 17 Feb 2023 12:00:54 -0000
@@ -1844,6 +1844,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))
@@ -1852,34 +1855,21 @@ sosetopt(struct socket *so, int level, i
if ((long)cnt <= 0)
cnt = 1;
switch (optname) {
-
case SO_SNDBUF:
- if (so->so_snd.sb_state & SS_CANTSENDMORE)
- return (EINVAL);
- if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
- sbreserve(so, &so->so_snd, cnt))
- return (ENOBUFS);
- 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))
return (EINVAL);
- if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
- sbreserve(so, &so->so_rcv, cnt))
+ if (sbcheckreserve(cnt, sb->sb_wat) ||
+ sbreserve(so, sb, cnt))
return (ENOBUFS);
- 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;
}
break;
@@ -1888,6 +1878,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;
@@ -1901,15 +1893,9 @@ sosetopt(struct socket *so, int level, i
return (EDOM);
if (nsecs == 0)
nsecs = INFSLP;
- 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;
+
break;
}
