[Scst-svn] SF.net SVN: scst:[5275] trunk/scst/src/scst_targ.c

[<vl...@us...>]

Revision: 5275
          http://sourceforge.net/p/scst/svn/5275
Author:   vlnb
Date:     2014-02-08 01:52:03 +0000 (Sat, 08 Feb 2014)
Log Message:
-----------
Fix spurious BUG when parse_type != SCST_USER_PARSE_STANDARD

Changeset 4224 introduced EXTRACHECKS for valid lba/data_len and state
at the end of the parsing phase of command processing.
However, the checks do not account for deferral of parsing to userland,
as occurs when SCST_USER_PARSE_CALL or SCST_USER_PARSE_EXCEPTION are specified.
In such cases the checks report errors on commands that userland has not yet
had an opportunity to parse.

NOTE: this includes a refactoring of the EXTRACHECKS to improve clarity.
      The rework is not exactly equivalent to the original code, but does
      conform to the comments describing the original code.
      Specifically, the original code would not trap an illegal command state
      unless there was also an illegal lba or data_len.

Signed-off-by: Steven J. Magnani <st...@di...>
with some improvements

Modified Paths:
--------------
    trunk/scst/src/scst_targ.c

Modified: trunk/scst/src/scst_targ.c
===================================================================
--- trunk/scst/src/scst_targ.c	2014-02-08 01:04:27 UTC (rev 5274)
+++ trunk/scst/src/scst_targ.c	2014-02-08 01:52:03 UTC (rev 5275)
@@ -941,23 +941,34 @@
 
 out:
 #ifdef CONFIG_SCST_EXTRACHECKS
-	/*
-	 * At this point either both lba and data_len must be initialized to
-	 * at least 0 for not data transfer commands, or cmd must be
-	 * completed (with an error) and have correct state set.
-	 */
-	if (unlikely((((cmd->lba == SCST_DEF_LBA_DATA_LEN) &&
-			!(cmd->op_flags & SCST_LBA_NOT_VALID)) ||
-		       (cmd->data_len == SCST_DEF_LBA_DATA_LEN)) &&
-		      (!cmd->completed ||
-		       (((cmd->state < SCST_CMD_STATE_PRE_XMIT_RESP) ||
-			 (cmd->state >= SCST_CMD_STATE_LAST_ACTIVE)) &&
-			(cmd->state != SCST_CMD_STATE_PREPROCESSING_DONE))))) {
-		PRINT_CRIT_ERROR("Not initialized data_len for going to "
-			"execute command or bad state (cmd %p, data_len %lld, "
-			"completed %d, state %d)", cmd,
-			(long long)cmd->data_len, cmd->completed, cmd->state);
-		sBUG();
+	if (unlikely(cmd->completed)) {
+		/* Command completed with error */
+		bool valid_state = (cmd->state == SCST_CMD_STATE_PREPROCESSING_DONE) ||
+				   ((cmd->state >= SCST_CMD_STATE_PRE_XMIT_RESP) &&
+				    (cmd->state < SCST_CMD_STATE_LAST_ACTIVE));
+
+		if (!valid_state) {
+			PRINT_CRIT_ERROR("Bad state for completed cmd "
+				"(cmd %p, state %d)", cmd, cmd->state);
+			sBUG();
+		}
+	} else if (cmd->state != SCST_CMD_STATE_PARSE) {
+		/*
+		 * Ready to execute. At this point both lba and data_len must
+		 * be initialized or marked non-applicable.
+		 */
+		bool bad_lba = (cmd->lba == SCST_DEF_LBA_DATA_LEN) &&
+			       !(cmd->op_flags & SCST_LBA_NOT_VALID);
+		bool bad_data_len = (cmd->data_len == SCST_DEF_LBA_DATA_LEN);
+
+		if (unlikely(bad_lba || bad_data_len)) {
+			PRINT_CRIT_ERROR("Uninitialized lba or data_len for "
+				"ready-to-execute command (cmd %p, lba %lld, "
+				"data_len %lld, state %d)", cmd,
+				(long long)cmd->lba, (long long)cmd->data_len,
+				cmd->state);
+			sBUG();
+		}
 	}
 #endif
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.