iscsi-scstd: Clarify result handling and connection cleanup

Checkpatch reports:
  ASSIGN_IN_IF: do not use assignment in if condition
  DEEP_INDENTATION: Too many leading tabs - consider code refactoring
  USE_NEGATIVE_ERRNO: return of an errno should typically be negative
    (ie: return -EAGAIN)

Separate status assignments from simple if conditions and preserve
short-circuit dispatch bounds. Leave the parameter-dispatch else-if
chains and their lookup precedence intact.

Flatten incoming-connection cleanup with early continues. Keep the CLOSE
case, kernel-owned destruction, session-count rollback and empty-session
release in their original order; no new ownership transfer is
introduced.

Return -EAGAIN for an empty nonblocking event queue. The drain loop
checks zero versus nonzero, while main-loop callers discard this result.
Retain EINTR retry and event draining behavior and document the return
convention.
This commit is contained in:
Gleb Chesnokov
2026-10-21 22:12:25 +03:00
parent c43ebb355b
commit dbc78782e1
4 changed files with 45 additions and 39 deletions
+8 -4
View File
@@ -520,7 +520,8 @@ static int handle_e_get_attr_value(int fd, const struct iscsi_kern_event *event)
p = buf;
pp = config_sep_string(&p);
if (!((idx = params_index_by_name(pp, target_keys)) < 0)) {
idx = params_index_by_name(pp, target_keys);
if (idx >= 0) {
if (target == NULL) {
log_error("Target expected for attr %s", pp);
res = -EINVAL;
@@ -850,7 +851,8 @@ static int handle_e_set_attr_value(int fd, const struct iscsi_kern_event *event)
p = buf;
pp = config_sep_string(&p);
if (!((idx = params_index_by_name(pp, target_keys)) < 0)) {
idx = params_index_by_name(pp, target_keys);
if (idx >= 0) {
struct iscsi_param params[target_key_last];
struct session *session;
@@ -1184,6 +1186,7 @@ out_free_server:
}
/* Return zero after handling an event, or -EAGAIN when no event is available. */
int handle_iscsi_events(int fd, bool wait)
{
struct session *session;
@@ -1200,9 +1203,10 @@ int handle_iscsi_events(int fd, bool wait)
STATIC_ASSERT(sizeof(event) % NLMSG_ALIGNTO == 0);
retry:
if ((rc = nl_read(fd, &event, sizeof(event), wait)) < 0) {
rc = nl_read(fd, &event, sizeof(event), wait);
if (rc < 0) {
if (errno == EAGAIN)
return EAGAIN;
return -EAGAIN;
if (errno == EINTR)
goto retry;
log_error("read netlink fd (%d) failed: %s", fd, strerror(errno));
+4 -3
View File
@@ -334,9 +334,9 @@ static int trgt_handle(int op, u32 set, u32 tid, char *params)
switch (op) {
case OP_NEW:
{
char *p = params;
char *p = params ? strchr(params, '=') : NULL;
if (!params || !(p = strchr(params, '='))) {
if (!p) {
fprintf(stderr, "Target name required\n");
err = -EINVAL;
goto out;
@@ -356,7 +356,8 @@ static int trgt_handle(int op, u32 set, u32 tid, char *params)
break;
case OP_UPDATE:
req.rcmnd = C_TRGT_UPDATE;
if ((err = parse_trgt_params(&req.u.trgt, params)) < 0)
err = parse_trgt_params(&req.u.trgt, params);
if (err < 0)
goto out;
break;
case OP_SHOW:
+31 -31
View File
@@ -760,6 +760,7 @@ static void event_loop(void)
for (i = 0; i < INCOMING_MAX; i++) {
struct connection *conn = incoming[i];
struct pollfd *pollfd = &poll_array[POLL_INCOMING + i];
struct session *sess;
if (!conn || !pollfd->revents)
continue;
@@ -768,39 +769,38 @@ static void event_loop(void)
event_conn(conn, pollfd);
if ((conn->state == STATE_CLOSE) ||
(conn->state == STATE_EXIT) ||
(conn->state == STATE_DROP)) {
struct session *sess = conn->sess;
if (conn->state != STATE_CLOSE &&
conn->state != STATE_EXIT &&
conn->state != STATE_DROP)
continue;
log_debug(1, "closing conn %p state=0x%x fd=%u",
conn, conn->state, pollfd->fd);
conn_free_pdu(conn);
close(pollfd->fd);
pollfd->fd = -1;
incoming[i] = NULL;
incoming_cnt--;
if (conn->state != STATE_CLOSE) {
if (conn->passed_to_kern) {
kernel_conn_destroy(conn->tid,
conn->sess->sid.id64,
conn->cid);
} else {
/*
* Check if session could not be established,
* but sessions count was already incremented
*/
if (!sess && conn->sessions_count_incremented)
conn->target->sessions_count--;
log_debug(1, "conn %p freed (sess %p, empty %d)",
conn, sess,
sess ? list_empty(&sess->conn_list) : -1);
conn_free(conn);
if (sess && list_empty(&sess->conn_list))
session_free(sess);
}
}
sess = conn->sess;
log_debug(1, "closing conn %p state=0x%x fd=%u",
conn, conn->state, pollfd->fd);
conn_free_pdu(conn);
close(pollfd->fd);
pollfd->fd = -1;
incoming[i] = NULL;
incoming_cnt--;
if (conn->state == STATE_CLOSE)
continue;
if (conn->passed_to_kern) {
kernel_conn_destroy(conn->tid, conn->sess->sid.id64, conn->cid);
continue;
}
/*
* The count may have been incremented before session
* establishment failed.
*/
if (!sess && conn->sessions_count_incremented)
conn->target->sessions_count--;
log_debug(1, "conn %p freed (sess %p, empty %d)",
conn, sess, sess ? list_empty(&sess->conn_list) : -1);
conn_free(conn);
if (sess && list_empty(&sess->conn_list))
session_free(sess);
}
}
}
+2 -1
View File
@@ -1162,7 +1162,8 @@ static void cmnd_exec_login(struct connection *conn)
switch (conn->state) {
case STATE_SECURITY:
case STATE_SECURITY_DONE:
if ((nsg_disagree = text_check_params(conn))) {
nsg_disagree = text_check_params(conn);
if (nsg_disagree) {
conn->state = STATE_LOGIN;
nsg = ISCSI_FLG_NSG_LOGIN;
break;