[PATCH 04/11] Port: removed useless msg->cancelled == 0 checks.

[Alejandro Colomar]

Hi Andrew,

On 6/16/22 03:00, Andrew Clayton wrote:
> In src/nxt_port_socket.c::nxt_port_read_msg_process() msg->cancelled is
> set to 0 and is not touched again.
> 
> However there are several checks for it being == 0 which are _always_
> true, so remove them.
> 
> I'm assuming that this is functioning as intended and that setting
> msg->cancelled to 0 is correct.
> 
> msg->cancelled was set to 0 unconditionally in commit e227fc9
> ("Introducing application and port shared memory queues."), so I guess
> the now redundant checks were simply missed for removal.

This patch makes sense, but then, I guess we should go further and 
remove that field completely?  I don't see it being used at all.

Cheers,

Alex


alx at asus5775:~/src/nginx/unit$ grep -rn '[.>]cancelled'
src/nxt_port_socket.c:1192:    msg->cancelled = 0;
src/nxt_port_socket.c:1202:        if (nxt_fast_path(fmsg->cancelled == 
0)) {
src/nxt_port_socket.c:1240:            if (msg->port_msg.mmap && 
msg->cancelled == 0) {
src/nxt_port_socket.c:1254:            if (nxt_fast_path(msg->cancelled 
== 0)) {
src/nxt_port_socket.c:1264:            if (nxt_fast_path(msg->cancelled 
== 0)) {
src/nxt_port_rpc.c:516:        msg.cancelled = 0;


alx at asus5775:~/src/nginx/unit$ grep -rn '\bcancelled\b' | grep -v -e nodejs
grep: build/src/nxt_process_title.o: binary file matches
src/nxt_router.c:542:    nxt_bool_t      cancelled;
src/nxt_router.c:555:        cancelled = 
nxt_app_queue_cancel(app_port->queue,
src/nxt_router.c:559:        if (cancelled) {
src/nxt_router.c:560:            nxt_debug(task, "stream #%uD: cancelled 
by router",
src/nxt_router.c:565:        cancelled = 0;
src/nxt_router.c:573:            b->is_port_mmap_sent = cancelled == 0;
src/nxt_router.c:581:    return cancelled;
src/nxt_router.c:4489:    /* TODO cancel message and return if cancelled. */
src/nxt_port_socket.c:1192:    msg->cancelled = 0;
src/nxt_port_socket.c:1202:        if (nxt_fast_path(fmsg->cancelled == 
0)) {
src/nxt_port_socket.c:1240:            if (msg->port_msg.mmap && 
msg->cancelled == 0) {
src/nxt_port_socket.c:1254:            if (nxt_fast_path(msg->cancelled 
== 0)) {
src/nxt_port_socket.c:1264:            if (nxt_fast_path(msg->cancelled 
== 0)) {
src/nxt_unit.c:6303:        nxt_unit_debug(NULL, "app_queue_recv: 
message cancelled");
src/nxt_port_rpc.c:516:        msg.cancelled = 0;
src/nxt_port.h:203:    nxt_bool_t          cancelled;

> ---
>   src/nxt_port_socket.c | 27 ++++++++++-----------------
>   1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/src/nxt_port_socket.c b/src/nxt_port_socket.c
> index 5752d5a..2649e69 100644
> --- a/src/nxt_port_socket.c
> +++ b/src/nxt_port_socket.c
> @@ -1237,7 +1237,7 @@ nxt_port_read_msg_process(nxt_task_t *task, nxt_port_t *port,
>       } else {
>           if (nxt_slow_path(msg->port_msg.mf != 0)) {
>   
> -            if (msg->port_msg.mmap && msg->cancelled == 0) {
> +            if (msg->port_msg.mmap) {
>                   nxt_port_mmap_read(task, msg);
>                   b = msg->buf;
>               }
> @@ -1251,25 +1251,18 @@ nxt_port_read_msg_process(nxt_task_t *task, nxt_port_t *port,
>               fmsg->port_msg.nf = 0;
>               fmsg->port_msg.mf = 0;
>   
> -            if (nxt_fast_path(msg->cancelled == 0)) {
> -                msg->buf = NULL;
> -                msg->fd[0] = -1;
> -                msg->fd[1] = -1;
> -                b = NULL;
> +            msg->buf = NULL;
> +            msg->fd[0] = -1;
> +            msg->fd[1] = -1;
> +            b = NULL;
>   
> -            } else {
> -                nxt_port_close_fds(msg->fd);
> -            }
>           } else {
> -            if (nxt_fast_path(msg->cancelled == 0)) {
> -
> -                if (msg->port_msg.mmap) {
> -                    nxt_port_mmap_read(task, msg);
> -                    b = msg->buf;
> -                }
> -
> -                port->handler(task, msg);
> +            if (msg->port_msg.mmap) {
> +                nxt_port_mmap_read(task, msg);
> +                b = msg->buf;
>               }
> +
> +            port->handler(task, msg);
>           }
>       }
>   

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>