gnrc_tcp: improvement: abort() - call #6997
Conversation
| { | ||
| gnrc_pktsnip_t *out_pkt = NULL; /* Outgoing packet */ | ||
| uint16_t seq_con = 0; /* Sequence number consumption of outgoing packet */ |
There was a problem hiding this comment.
scope of both variables can be reduced, move into if below.
Docu of such internal variables is not necessary, especially when obvious 😉
| return -EOPNOTSUPP; | ||
|
|
||
| /* A reset must be sent in case the TCB state is in one of those cases */ | ||
| if (tcb->state == FSM_STATE_SYN_RCVD || tcb->state == FSM_STATE_ESTABLISHED || |
There was a problem hiding this comment.
this might look nicer:
switch (tcb->state) {
case FSM_STATE_SYN_RECV:
case FSM_STATE_ESTABLISHED:
...
case FSM_STATE_CLOSE_WAIT:
gnrc_pktsnip_t *out_pkt = NULL;
uint16_t seq_con = 0;
/* Send RST packet without retransmit */
_pkt_build(tcb, &out_pkt, &seq_con, MSK_RST, tcb->snd_nxt, tcb->rcv_nxt, NULL, 0);
_pkt_send(tcb, out_pkt, seq_con, false);
break;
default:
break;
}
There was a problem hiding this comment.
Actually I though about it, but it would look inconsistent with all the other cases where I do the state checking.
There was a problem hiding this comment.
might be something to consider for later on
There was a problem hiding this comment.
Hmm is a switch case more efficient? In one of my earlier semesters, I think that a professor claimed that the switch case solution is in those cases more efficient.
There was a problem hiding this comment.
yeah right, not sure if that matters here - and who knows what modern compiler optimize away anyway 😉 My point rather was, that it might be more readable and extensible (if required in the future) using switch-case over if-else.
But as said, not in this PR - as it is not related ...
sys/include/net/gnrc/tcp.h
Outdated
| * | ||
| * @returns Zero on success. | ||
| */ | ||
| int gnrc_tcp_abort(gnrc_tcp_tcb_t *tcb); |
There was a problem hiding this comment.
why not void as it only returns 0 anyway?
There was a problem hiding this comment.
Good point, I should change it on close call too then.
92f9648 to
8868204
Compare
|
ACK, though changing scope of variables is unrelated to the rest. Let's see what Murdock has to say ... |
|
please squash |
532d1ba to
b08b12e
Compare
|
Done |
This PR introduces the abort() function call to GNRC TCP.
In contrast to gnrc_close() (terminates a connection gracefully) gnrc_abort() resets a function immediately.