socket: make socket_* functions static
PATCH 1/1 socket: make socket_* functions static
stream_buf_init(), stream_buf_close() and stream_buf_added() are only used within socket.c, therefore there is noneed to have them declared in socket.h.
Make them static and remove useless declarations. This change required some re-ordering of the functions to ensure they were defined before being used, however, no this is just a copy/paste and no function change has been introduced.
Signed-off-by Antonio Quartulli <[email protected]...>
--- src/openvpn/socket.c | 362 +++++++++++++++++++++---------------------- src/openvpn/socket.h | 15 -- 2 files changed, 181 insertions(+), 196 deletions(-)
src/openvpn/socket.c +181 -181
@@ -1491,6 +1491,187 @@ done:gc_free (&gc); } +/* +* Stream buffer functions, used to packetize a TCP +* stream connection. +*/ + +static inline void +stream_buf_reset (struct stream_buf *sb) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: RESET" ); + sb->residual_fully_formed = false; + sb->buf = sb->buf_init; +buf_reset (&sb->next); + sb->len =-1 ; +} + +static void +stream_buf_init (struct stream_buf *sb, +struct buffer *buf, +const unsigned int sockflags, +const int proto) +{ + sb->buf_init = *buf; + sb->maxlen = sb->buf_init.len; + sb->buf_init.len =0 ; + sb->residual =alloc_buf (sb->maxlen); + sb->error = false; +#if PORT_SHARE + sb->port_share_state = ((sockflags &SF_PORT_SHARE ) && (proto ==PROTO_TCP_SERVER )) + ?PS_ENABLED + :PS_DISABLED ; +#endif +stream_buf_reset (sb); + +dmsg (D_STREAM_DEBUG ,"STREAM: INIT maxlen=%d" , sb->maxlen); +} + +static void +stream_buf_close (struct stream_buf *sb) +{ +free_buf (&sb->residual); +} + +static inline void +stream_buf_set_next (struct stream_buf *sb) +{ +/* set up 'next' for next i/o read */ + sb->next = sb->buf; + sb->next.offset = sb->buf.offset + sb->buf.len; + sb->next.len = (sb->len >=0 ? sb->len : sb->maxlen) - sb->buf.len; +dmsg (D_STREAM_DEBUG ,"STREAM: SET NEXT, buf=[%d,%d] next=[%d,%d] len=%d maxlen=%d" , + sb->buf.offset, sb->buf.len, + sb->next.offset, sb->next.len, + sb->len, sb->maxlen); +ASSERT (sb->next.len >0 ); +ASSERT (buf_safe (&sb->buf, sb->next.len)); +} + +static inline void +stream_buf_get_final (struct stream_buf *sb,struct buffer *buf) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: GET FINAL len=%d" , +buf_defined (&sb->buf) ? sb->buf.len :-1 ); +ASSERT (buf_defined (&sb->buf)); + *buf = sb->buf; +} + +static inline void +stream_buf_get_next (struct stream_buf *sb,struct buffer *buf) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: GET NEXT len=%d" , +buf_defined (&sb->next) ? sb->next.len :-1 ); +ASSERT (buf_defined (&sb->next)); + *buf = sb->next; +} + +static bool +stream_buf_added (struct stream_buf *sb, +int length_added) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: ADD length_added=%d" , length_added); +if (length_added >0 ) + { + sb->buf.len += length_added; + } + +/* if length unknown, see if we can get the length prefix from +* the head of the buffer */ +if (sb->len <0 && sb->buf.len >= (int )sizeof (packet_size_type)) + { +packet_size_type net_size; + +#if PORT_SHARE +if (sb->port_share_state ==PS_ENABLED ) + { +if (!is_openvpn_protocol (&sb->buf)) + { +msg (D_STREAM_ERRORS ,"Non-OpenVPN client protocol detected" ); + sb->port_share_state =PS_FOREIGN ; + sb->error = true; +return false; + } +else + { + sb->port_share_state =PS_DISABLED ; + } + } +#endif + +ASSERT (buf_read (&sb->buf, &net_size,sizeof (net_size))); + sb->len =ntohps (net_size); + +if (sb->len <1 || sb->len > sb->maxlen) + { +msg (M_WARN ,"WARNING: Bad encapsulated packet length from peer (%d), which must be > 0 and <= %d -- please ensure that --tun-mtu or --link-mtu is equal on both peers -- this condition could also indicate a possible active attack on the TCP link -- [Attempting restart...]" , sb->len, sb->maxlen); +stream_buf_reset (sb); + sb->error = true; +return false; + } + } + +/* is our incoming packet fully read? */ +if (sb->len >0 && sb->buf.len >= sb->len) + { +/* save any residual data that's part of the next packet */ +ASSERT (buf_init (&sb->residual,0 )); +if (sb->buf.len > sb->len) + { +ASSERT (buf_copy_excess (&sb->residual, &sb->buf, sb->len)); + } +dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned TRUE, buf_len=%d, residual_len=%d" , +BLEN (&sb->buf), +BLEN (&sb->residual)); +return true; + } +else + { +dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned FALSE (have=%d need=%d)" , sb->buf.len, sb->len); +stream_buf_set_next (sb); +return false; + } +} + +bool +stream_buf_read_setup_dowork (struct link_socket *sock) +{ +if (sock->stream_buf.residual.len && !sock->stream_buf.residual_fully_formed) + { +ASSERT (buf_copy (&sock->stream_buf.buf, &sock->stream_buf.residual)); +ASSERT (buf_init (&sock->stream_buf.residual,0 )); + sock->stream_buf.residual_fully_formed =stream_buf_added (&sock->stream_buf,0 ); +dmsg (D_STREAM_DEBUG ,"STREAM: RESIDUAL FULLY FORMED [%s], len=%d" , + sock->stream_buf.residual_fully_formed ?"YES" :"NO" , + sock->stream_buf.residual.len); + } + +if (!sock->stream_buf.residual_fully_formed) + { +stream_buf_set_next (&sock->stream_buf); + } +return !sock->stream_buf.residual_fully_formed; +} + +/* +* The listen event is a special event whose sole purpose is +* to tell us that there's a new incoming connection on a +* TCP socket, for use in server mode. +*/ +event_t +socket_listen_event_handle (struct link_socket *s) +{ +#ifdef _WIN32 +if (!defined_net_event_win32 (&s->listen_handle)) + { +init_net_event_win32 (&s->listen_handle,FD_ACCEPT , s->sd,0 ); + } +return &s->listen_handle; +#else /* ifdef _WIN32 */ +return s->sd; +#endif +} +/* For stream protocols, allocate a buffer to build up packet. * Called after frame has been finalized. */
@@ -2485,187 +2666,6 @@ socket_stat(const struct link_socket *s, unsigned int rwflags, struct gc_arena *return BSTR (&out); } -/* -* Stream buffer functions, used to packetize a TCP -* stream connection. -*/ - -static inline void -stream_buf_reset (struct stream_buf *sb) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: RESET" ); - sb->residual_fully_formed = false; - sb->buf = sb->buf_init; -buf_reset (&sb->next); - sb->len =-1 ; -} - -void -stream_buf_init (struct stream_buf *sb, -struct buffer *buf, -const unsigned int sockflags, -const int proto) -{ - sb->buf_init = *buf; - sb->maxlen = sb->buf_init.len; - sb->buf_init.len =0 ; - sb->residual =alloc_buf (sb->maxlen); - sb->error = false; -#if PORT_SHARE - sb->port_share_state = ((sockflags &SF_PORT_SHARE ) && (proto ==PROTO_TCP_SERVER )) - ?PS_ENABLED - :PS_DISABLED ; -#endif -stream_buf_reset (sb); - -dmsg (D_STREAM_DEBUG ,"STREAM: INIT maxlen=%d" , sb->maxlen); -} - -static inline void -stream_buf_set_next (struct stream_buf *sb) -{ -/* set up 'next' for next i/o read */ - sb->next = sb->buf; - sb->next.offset = sb->buf.offset + sb->buf.len; - sb->next.len = (sb->len >=0 ? sb->len : sb->maxlen) - sb->buf.len; -dmsg (D_STREAM_DEBUG ,"STREAM: SET NEXT, buf=[%d,%d] next=[%d,%d] len=%d maxlen=%d" , - sb->buf.offset, sb->buf.len, - sb->next.offset, sb->next.len, - sb->len, sb->maxlen); -ASSERT (sb->next.len >0 ); -ASSERT (buf_safe (&sb->buf, sb->next.len)); -} - -static inline void -stream_buf_get_final (struct stream_buf *sb,struct buffer *buf) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: GET FINAL len=%d" , -buf_defined (&sb->buf) ? sb->buf.len :-1 ); -ASSERT (buf_defined (&sb->buf)); - *buf = sb->buf; -} - -static inline void -stream_buf_get_next (struct stream_buf *sb,struct buffer *buf) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: GET NEXT len=%d" , -buf_defined (&sb->next) ? sb->next.len :-1 ); -ASSERT (buf_defined (&sb->next)); - *buf = sb->next; -} - -bool -stream_buf_read_setup_dowork (struct link_socket *sock) -{ -if (sock->stream_buf.residual.len && !sock->stream_buf.residual_fully_formed) - { -ASSERT (buf_copy (&sock->stream_buf.buf, &sock->stream_buf.residual)); -ASSERT (buf_init (&sock->stream_buf.residual,0 )); - sock->stream_buf.residual_fully_formed =stream_buf_added (&sock->stream_buf,0 ); -dmsg (D_STREAM_DEBUG ,"STREAM: RESIDUAL FULLY FORMED [%s], len=%d" , - sock->stream_buf.residual_fully_formed ?"YES" :"NO" , - sock->stream_buf.residual.len); - } - -if (!sock->stream_buf.residual_fully_formed) - { -stream_buf_set_next (&sock->stream_buf); - } -return !sock->stream_buf.residual_fully_formed; -} - -bool -stream_buf_added (struct stream_buf *sb, -int length_added) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: ADD length_added=%d" , length_added); -if (length_added >0 ) - { - sb->buf.len += length_added; - } - -/* if length unknown, see if we can get the length prefix from -* the head of the buffer */ -if (sb->len <0 && sb->buf.len >= (int )sizeof (packet_size_type)) - { -packet_size_type net_size; - -#if PORT_SHARE -if (sb->port_share_state ==PS_ENABLED ) - { -if (!is_openvpn_protocol (&sb->buf)) - { -msg (D_STREAM_ERRORS ,"Non-OpenVPN client protocol detected" ); - sb->port_share_state =PS_FOREIGN ; - sb->error = true; -return false; - } -else - { - sb->port_share_state =PS_DISABLED ; - } - } -#endif - -ASSERT (buf_read (&sb->buf, &net_size,sizeof (net_size))); - sb->len =ntohps (net_size); - -if (sb->len <1 || sb->len > sb->maxlen) - { -msg (M_WARN ,"WARNING: Bad encapsulated packet length from peer (%d), which must be > 0 and <= %d -- please ensure that --tun-mtu or --link-mtu is equal on both peers -- this condition could also indicate a possible active attack on the TCP link -- [Attempting restart...]" , sb->len, sb->maxlen); -stream_buf_reset (sb); - sb->error = true; -return false; - } - } - -/* is our incoming packet fully read? */ -if (sb->len >0 && sb->buf.len >= sb->len) - { -/* save any residual data that's part of the next packet */ -ASSERT (buf_init (&sb->residual,0 )); -if (sb->buf.len > sb->len) - { -ASSERT (buf_copy_excess (&sb->residual, &sb->buf, sb->len)); - } -dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned TRUE, buf_len=%d, residual_len=%d" , -BLEN (&sb->buf), -BLEN (&sb->residual)); -return true; - } -else - { -dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned FALSE (have=%d need=%d)" , sb->buf.len, sb->len); -stream_buf_set_next (sb); -return false; - } -} - -void -stream_buf_close (struct stream_buf *sb) -{ -free_buf (&sb->residual); -} - -/* -* The listen event is a special event whose sole purpose is -* to tell us that there's a new incoming connection on a -* TCP socket, for use in server mode. -*/ -event_t -socket_listen_event_handle (struct link_socket *s) -{ -#ifdef _WIN32 -if (!defined_net_event_win32 (&s->listen_handle)) - { -init_net_event_win32 (&s->listen_handle,FD_ACCEPT , s->sd,0 ); - } -return &s->listen_handle; -#else /* ifdef _WIN32 */ -return s->sd; -#endif -} -/* * Format IP addresses in ascii */
src/openvpn/socket.h +0 -15
@@ -994,21 +994,6 @@ link_socket_set_outgoing_addr(const struct buffer *buf, } } -/* -* Stream buffer handling -- stream_buf is a helper class -* to assist in the packetization of stream transport protocols -* such as TCP. -*/ - -void stream_buf_init (struct stream_buf *sb, -struct buffer *buf, -const unsigned int sockflags, -const int proto); - -void stream_buf_close (struct stream_buf *sb); - -bool stream_buf_added (struct stream_buf *sb,int length_added); -static inline bool stream_buf_read_setup (struct link_socket *sock) {
Signature · 1 line
2.18.0
PATCH v2 1/1 socket: make stream_buf_* functions static
stream_buf_init(), stream_buf_close() and stream_buf_added() are only used within socket.c, therefore there is noneed to have them declared in socket.h.
Make them static and remove useless declarations. This change required some re-ordering of the functions to ensure they were defined before being used, however, no this is just a copy/paste and no function change has been introduced.
Signed-off-by Antonio Quartulli <[email protected]...>
---
v2:
- fix commit subject
src/openvpn/socket.c | 362 +++++++++++++++++++++---------------------- src/openvpn/socket.h | 15 -- 2 files changed, 181 insertions(+), 196 deletions(-)
src/openvpn/socket.c +181 -181
@@ -1491,6 +1491,187 @@ done:gc_free (&gc); } +/* +* Stream buffer functions, used to packetize a TCP +* stream connection. +*/ + +static inline void +stream_buf_reset (struct stream_buf *sb) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: RESET" ); + sb->residual_fully_formed = false; + sb->buf = sb->buf_init; +buf_reset (&sb->next); + sb->len =-1 ; +} + +static void +stream_buf_init (struct stream_buf *sb, +struct buffer *buf, +const unsigned int sockflags, +const int proto) +{ + sb->buf_init = *buf; + sb->maxlen = sb->buf_init.len; + sb->buf_init.len =0 ; + sb->residual =alloc_buf (sb->maxlen); + sb->error = false; +#if PORT_SHARE + sb->port_share_state = ((sockflags &SF_PORT_SHARE ) && (proto ==PROTO_TCP_SERVER )) + ?PS_ENABLED + :PS_DISABLED ; +#endif +stream_buf_reset (sb); + +dmsg (D_STREAM_DEBUG ,"STREAM: INIT maxlen=%d" , sb->maxlen); +} + +static void +stream_buf_close (struct stream_buf *sb) +{ +free_buf (&sb->residual); +} + +static inline void +stream_buf_set_next (struct stream_buf *sb) +{ +/* set up 'next' for next i/o read */ + sb->next = sb->buf; + sb->next.offset = sb->buf.offset + sb->buf.len; + sb->next.len = (sb->len >=0 ? sb->len : sb->maxlen) - sb->buf.len; +dmsg (D_STREAM_DEBUG ,"STREAM: SET NEXT, buf=[%d,%d] next=[%d,%d] len=%d maxlen=%d" , + sb->buf.offset, sb->buf.len, + sb->next.offset, sb->next.len, + sb->len, sb->maxlen); +ASSERT (sb->next.len >0 ); +ASSERT (buf_safe (&sb->buf, sb->next.len)); +} + +static inline void +stream_buf_get_final (struct stream_buf *sb,struct buffer *buf) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: GET FINAL len=%d" , +buf_defined (&sb->buf) ? sb->buf.len :-1 ); +ASSERT (buf_defined (&sb->buf)); + *buf = sb->buf; +} + +static inline void +stream_buf_get_next (struct stream_buf *sb,struct buffer *buf) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: GET NEXT len=%d" , +buf_defined (&sb->next) ? sb->next.len :-1 ); +ASSERT (buf_defined (&sb->next)); + *buf = sb->next; +} + +static bool +stream_buf_added (struct stream_buf *sb, +int length_added) +{ +dmsg (D_STREAM_DEBUG ,"STREAM: ADD length_added=%d" , length_added); +if (length_added >0 ) + { + sb->buf.len += length_added; + } + +/* if length unknown, see if we can get the length prefix from +* the head of the buffer */ +if (sb->len <0 && sb->buf.len >= (int )sizeof (packet_size_type)) + { +packet_size_type net_size; + +#if PORT_SHARE +if (sb->port_share_state ==PS_ENABLED ) + { +if (!is_openvpn_protocol (&sb->buf)) + { +msg (D_STREAM_ERRORS ,"Non-OpenVPN client protocol detected" ); + sb->port_share_state =PS_FOREIGN ; + sb->error = true; +return false; + } +else + { + sb->port_share_state =PS_DISABLED ; + } + } +#endif + +ASSERT (buf_read (&sb->buf, &net_size,sizeof (net_size))); + sb->len =ntohps (net_size); + +if (sb->len <1 || sb->len > sb->maxlen) + { +msg (M_WARN ,"WARNING: Bad encapsulated packet length from peer (%d), which must be > 0 and <= %d -- please ensure that --tun-mtu or --link-mtu is equal on both peers -- this condition could also indicate a possible active attack on the TCP link -- [Attempting restart...]" , sb->len, sb->maxlen); +stream_buf_reset (sb); + sb->error = true; +return false; + } + } + +/* is our incoming packet fully read? */ +if (sb->len >0 && sb->buf.len >= sb->len) + { +/* save any residual data that's part of the next packet */ +ASSERT (buf_init (&sb->residual,0 )); +if (sb->buf.len > sb->len) + { +ASSERT (buf_copy_excess (&sb->residual, &sb->buf, sb->len)); + } +dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned TRUE, buf_len=%d, residual_len=%d" , +BLEN (&sb->buf), +BLEN (&sb->residual)); +return true; + } +else + { +dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned FALSE (have=%d need=%d)" , sb->buf.len, sb->len); +stream_buf_set_next (sb); +return false; + } +} + +bool +stream_buf_read_setup_dowork (struct link_socket *sock) +{ +if (sock->stream_buf.residual.len && !sock->stream_buf.residual_fully_formed) + { +ASSERT (buf_copy (&sock->stream_buf.buf, &sock->stream_buf.residual)); +ASSERT (buf_init (&sock->stream_buf.residual,0 )); + sock->stream_buf.residual_fully_formed =stream_buf_added (&sock->stream_buf,0 ); +dmsg (D_STREAM_DEBUG ,"STREAM: RESIDUAL FULLY FORMED [%s], len=%d" , + sock->stream_buf.residual_fully_formed ?"YES" :"NO" , + sock->stream_buf.residual.len); + } + +if (!sock->stream_buf.residual_fully_formed) + { +stream_buf_set_next (&sock->stream_buf); + } +return !sock->stream_buf.residual_fully_formed; +} + +/* +* The listen event is a special event whose sole purpose is +* to tell us that there's a new incoming connection on a +* TCP socket, for use in server mode. +*/ +event_t +socket_listen_event_handle (struct link_socket *s) +{ +#ifdef _WIN32 +if (!defined_net_event_win32 (&s->listen_handle)) + { +init_net_event_win32 (&s->listen_handle,FD_ACCEPT , s->sd,0 ); + } +return &s->listen_handle; +#else /* ifdef _WIN32 */ +return s->sd; +#endif +} +/* For stream protocols, allocate a buffer to build up packet. * Called after frame has been finalized. */
@@ -2485,187 +2666,6 @@ socket_stat(const struct link_socket *s, unsigned int rwflags, struct gc_arena *return BSTR (&out); } -/* -* Stream buffer functions, used to packetize a TCP -* stream connection. -*/ - -static inline void -stream_buf_reset (struct stream_buf *sb) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: RESET" ); - sb->residual_fully_formed = false; - sb->buf = sb->buf_init; -buf_reset (&sb->next); - sb->len =-1 ; -} - -void -stream_buf_init (struct stream_buf *sb, -struct buffer *buf, -const unsigned int sockflags, -const int proto) -{ - sb->buf_init = *buf; - sb->maxlen = sb->buf_init.len; - sb->buf_init.len =0 ; - sb->residual =alloc_buf (sb->maxlen); - sb->error = false; -#if PORT_SHARE - sb->port_share_state = ((sockflags &SF_PORT_SHARE ) && (proto ==PROTO_TCP_SERVER )) - ?PS_ENABLED - :PS_DISABLED ; -#endif -stream_buf_reset (sb); - -dmsg (D_STREAM_DEBUG ,"STREAM: INIT maxlen=%d" , sb->maxlen); -} - -static inline void -stream_buf_set_next (struct stream_buf *sb) -{ -/* set up 'next' for next i/o read */ - sb->next = sb->buf; - sb->next.offset = sb->buf.offset + sb->buf.len; - sb->next.len = (sb->len >=0 ? sb->len : sb->maxlen) - sb->buf.len; -dmsg (D_STREAM_DEBUG ,"STREAM: SET NEXT, buf=[%d,%d] next=[%d,%d] len=%d maxlen=%d" , - sb->buf.offset, sb->buf.len, - sb->next.offset, sb->next.len, - sb->len, sb->maxlen); -ASSERT (sb->next.len >0 ); -ASSERT (buf_safe (&sb->buf, sb->next.len)); -} - -static inline void -stream_buf_get_final (struct stream_buf *sb,struct buffer *buf) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: GET FINAL len=%d" , -buf_defined (&sb->buf) ? sb->buf.len :-1 ); -ASSERT (buf_defined (&sb->buf)); - *buf = sb->buf; -} - -static inline void -stream_buf_get_next (struct stream_buf *sb,struct buffer *buf) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: GET NEXT len=%d" , -buf_defined (&sb->next) ? sb->next.len :-1 ); -ASSERT (buf_defined (&sb->next)); - *buf = sb->next; -} - -bool -stream_buf_read_setup_dowork (struct link_socket *sock) -{ -if (sock->stream_buf.residual.len && !sock->stream_buf.residual_fully_formed) - { -ASSERT (buf_copy (&sock->stream_buf.buf, &sock->stream_buf.residual)); -ASSERT (buf_init (&sock->stream_buf.residual,0 )); - sock->stream_buf.residual_fully_formed =stream_buf_added (&sock->stream_buf,0 ); -dmsg (D_STREAM_DEBUG ,"STREAM: RESIDUAL FULLY FORMED [%s], len=%d" , - sock->stream_buf.residual_fully_formed ?"YES" :"NO" , - sock->stream_buf.residual.len); - } - -if (!sock->stream_buf.residual_fully_formed) - { -stream_buf_set_next (&sock->stream_buf); - } -return !sock->stream_buf.residual_fully_formed; -} - -bool -stream_buf_added (struct stream_buf *sb, -int length_added) -{ -dmsg (D_STREAM_DEBUG ,"STREAM: ADD length_added=%d" , length_added); -if (length_added >0 ) - { - sb->buf.len += length_added; - } - -/* if length unknown, see if we can get the length prefix from -* the head of the buffer */ -if (sb->len <0 && sb->buf.len >= (int )sizeof (packet_size_type)) - { -packet_size_type net_size; - -#if PORT_SHARE -if (sb->port_share_state ==PS_ENABLED ) - { -if (!is_openvpn_protocol (&sb->buf)) - { -msg (D_STREAM_ERRORS ,"Non-OpenVPN client protocol detected" ); - sb->port_share_state =PS_FOREIGN ; - sb->error = true; -return false; - } -else - { - sb->port_share_state =PS_DISABLED ; - } - } -#endif - -ASSERT (buf_read (&sb->buf, &net_size,sizeof (net_size))); - sb->len =ntohps (net_size); - -if (sb->len <1 || sb->len > sb->maxlen) - { -msg (M_WARN ,"WARNING: Bad encapsulated packet length from peer (%d), which must be > 0 and <= %d -- please ensure that --tun-mtu or --link-mtu is equal on both peers -- this condition could also indicate a possible active attack on the TCP link -- [Attempting restart...]" , sb->len, sb->maxlen); -stream_buf_reset (sb); - sb->error = true; -return false; - } - } - -/* is our incoming packet fully read? */ -if (sb->len >0 && sb->buf.len >= sb->len) - { -/* save any residual data that's part of the next packet */ -ASSERT (buf_init (&sb->residual,0 )); -if (sb->buf.len > sb->len) - { -ASSERT (buf_copy_excess (&sb->residual, &sb->buf, sb->len)); - } -dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned TRUE, buf_len=%d, residual_len=%d" , -BLEN (&sb->buf), -BLEN (&sb->residual)); -return true; - } -else - { -dmsg (D_STREAM_DEBUG ,"STREAM: ADD returned FALSE (have=%d need=%d)" , sb->buf.len, sb->len); -stream_buf_set_next (sb); -return false; - } -} - -void -stream_buf_close (struct stream_buf *sb) -{ -free_buf (&sb->residual); -} - -/* -* The listen event is a special event whose sole purpose is -* to tell us that there's a new incoming connection on a -* TCP socket, for use in server mode. -*/ -event_t -socket_listen_event_handle (struct link_socket *s) -{ -#ifdef _WIN32 -if (!defined_net_event_win32 (&s->listen_handle)) - { -init_net_event_win32 (&s->listen_handle,FD_ACCEPT , s->sd,0 ); - } -return &s->listen_handle; -#else /* ifdef _WIN32 */ -return s->sd; -#endif -} -/* * Format IP addresses in ascii */
src/openvpn/socket.h +0 -15
@@ -994,21 +994,6 @@ link_socket_set_outgoing_addr(const struct buffer *buf, } } -/* -* Stream buffer handling -- stream_buf is a helper class -* to assist in the packetization of stream transport protocols -* such as TCP. -*/ - -void stream_buf_init (struct stream_buf *sb, -struct buffer *buf, -const unsigned int sockflags, -const int proto); - -void stream_buf_close (struct stream_buf *sb); - -bool stream_buf_added (struct stream_buf *sb,int length_added); -static inline bool stream_buf_read_setup (struct link_socket *sock) {
Signature · 1 line
2.18.0
Re: [Openvpn-devel] [PATCH v2] socket: make stream_buf_* functions static
Hi,
Quoted 9 lines · Antonio Quartulli
stream_buf_init(),stream_buf_close()andstream_buf_added()are only used withinsocket.c, therefore there is noneed to have them declared insocket.h.Make them static and remove useless declarations. This change required some re-ordering of the functions to ensure they were defined before being used, however, no this is just a copy/paste and no function change has been introduced.
I'm not so happy about the large code move-around here, for small benefit (code move-around breaks "git blame" to see which commit introduced something, and why).
Why not just add prototypes at the top?
gert
Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]...Re: [Openvpn-devel] [PATCH v2] socket: make stream_buf_* functions static
Hi,
Quoted 13 lines · Gert Doering
Hi,
Quoted 9 lines · Antonio Quartulli
stream_buf_init(),stream_buf_close()andstream_buf_added()are only used withinsocket.c, therefore there is noneed to have them declared insocket.h.Make them static and remove useless declarations. This change required some re-ordering of the functions to ensure they were defined before being used, however, no this is just a copy/paste and no function change has been introduced.
Quoted 5 lines
I'm not so happy about the large code move-around here, for small benefit (code move-around breaks "git blame" to see which commit introduced something, and why).
Why not just add prototypes at the top?
Because I wanted the result to be clean code :)
Imho, prototypes should be used only if really needed (i.e. when re-ordering is not an option).
The information about what/why was introduced can still be extracted with a "git log -S$something_here".
I wouldn't make such a big deal of "breaking git blame" if the changes slowly move towards cleaner code.
Actually some functions did not strictly needed to be moved, but I wanted all the stream_buf helpers to be in the same place and not scattered along socket.c. This can be changed if you think it's too invasive.
my 2 cents.
Cheers,
Signature · 1 line
Antonio Quartulli
Re: [Openvpn-devel] [PATCH v2] socket: make stream_buf_* functions static
Hi,
Quoted 3 lines · Antonio Quartulli
Quoted 1 line
Why not just add prototypes at the top?
Because I wanted the result to be clean code :)
Prototypes are clean code :-)
Quoted 5 lines
Imho, prototypes should be used only if really needed (i.e. when re-ordering is not an option).
The information about what/why was introduced can still be extracted with a "git log -S$something_here".
Won't help if I want to see how a particular line came to be - and this is something relevant if you try to understand why "openvpn used to do things differently" in earlier versions.
Quoted 2 lines
I wouldn't make such a big deal of "breaking git blame" if the changes slowly move towards cleaner code.
I'm not objecting if it moves "towards cleaner code" (like the 2.4 reformatting, or moving code blobs to smaller units) - but "just move it around in the same source file" is not achieving much cleaning, so I consider the negative aspects to be stronger in that case.
Quoted 4 lines
Actually some functions did not strictly needed to be moved, but I wanted all the
stream_bufhelpers to be in the same place and not scattered alongsocket.c. This can be changed if you think it's too invasive.
I do.
gert
Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]...PATCH v3 1/1 socket: make stream_buf_* functions static
In reply to Antonio Quartullistream_buf_init(), stream_buf_close() and stream_buf_added() are only used within socket.c, therefore there is noneed to have them declared in socket.h.
Make them static and remove useless declarations. This change reuired adding function prototypes in socket.c to avoid useless code re-ordering.
Signed-off-by Antonio Quartulli <[email protected]...>
---
v2:
- fix commit subject
v3:
- move functions back to their original locations and add prototypes before the first usage of the three functions
--- src/openvpn/socket.c | 22 +++++++++++++++++++--- src/openvpn/socket.h | 15 --------------- 2 files changed, 19 insertions(+), 18 deletions(-)
src/openvpn/socket.c +19 -3
@@ -1491,6 +1491,22 @@ done:gc_free (&gc); } +/* +* Stream buffer handling prototypes -- stream_buf is a helper class +* to assist in the packetization of stream transport protocols +* such as TCP. +*/ + +static void +stream_buf_init (struct stream_buf *sb,struct buffer *buf, +const unsigned int sockflags,const int proto); + +static void +stream_buf_close (struct stream_buf *sb); + +static bool +stream_buf_added (struct stream_buf *sb,int length_added); +/* For stream protocols, allocate a buffer to build up packet. * Called after frame has been finalized. */
@@ -2500,7 +2516,7 @@ stream_buf_reset(struct stream_buf *sb) sb->len =-1 ; } -void +static void stream_buf_init(struct stream_buf *sb, struct buffer *buf, const unsigned int sockflags,
@@ -2574,7 +2590,7 @@ stream_buf_read_setup_dowork(struct link_socket *sock)return !sock->stream_buf.residual_fully_formed; } -bool +static bool stream_buf_added (struct stream_buf *sb,int length_added) {
@@ -2641,7 +2657,7 @@ stream_buf_added(struct stream_buf *sb, } } -void +static void stream_buf_close (struct stream_buf *sb) {free_buf (&sb->residual);
src/openvpn/socket.h +0 -15
@@ -994,21 +994,6 @@ link_socket_set_outgoing_addr(const struct buffer *buf, } } -/* -* Stream buffer handling -- stream_buf is a helper class -* to assist in the packetization of stream transport protocols -* such as TCP. -*/ - -void stream_buf_init (struct stream_buf *sb, -struct buffer *buf, -const unsigned int sockflags, -const int proto); - -void stream_buf_close (struct stream_buf *sb); - -bool stream_buf_added (struct stream_buf *sb,int length_added); -static inline bool stream_buf_read_setup (struct link_socket *sock) {
Signature · 1 line
2.18.0
[Openvpn-devel] [PATCH applied] Re: socket: make stream_buf_* functions static
Acked-by Gert Doering <[email protected]...>
"Having local code self-contained is good" - visually tested, and t_client test runs. Thanks for bearing with me :)
Your patch has been applied to the master branch.
commit 62c0455e0fd48b4640251dc879c25d4f66ae3ed2
Author: Antonio Quartulli Date: Thu Jul 12 09:29:55 2018 +0800
socket: make stream_buf_* functions static
Signed-off-by: Antonio Quartulli <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg17246.html
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering