Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1581598 > unrolled thread

[PATCHv3 2/4] block/sed: Add helper to qualify response tokens

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCHv3 2/4] block/sed: Add helper to qualify response tokens Jon Derrick <jonathan.derrick@intel.com> - 2017-02-15 20:50 +0100
    Re: [PATCHv3 2/4] block/sed: Add helper to qualify response tokens Scott Bauer <scott.bauer@intel.com> - 2017-02-15 22:30 +0100

#1581598 — [PATCHv3 2/4] block/sed: Add helper to qualify response tokens

Add helper which verifies the response token is valid and matches the
expected value. Merges token_type and response_get_token.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 block/sed-opal.c | 61 +++++++++++++++++++++++---------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 77623ad..d6dd604 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -591,48 +591,25 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
 	return 0;
 }
 
-static enum opal_response_token token_type(const struct parsed_resp *resp,
-					   int n)
+static const struct opal_resp_tok *response_get_token(
+				const struct parsed_resp *resp,
+				int n)
 {
 	const struct opal_resp_tok *tok;
 
 	if (n >= resp->num) {
 		pr_err("Token number doesn't exist: %d, resp: %d\n",
 		       n, resp->num);
-		return OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
 	tok = &resp->toks[n];
 	if (tok->len == 0) {
 		pr_err("Token length must be non-zero\n");
-		return OPAL_DTA_TOKENID_INVALID;
+		return ERR_PTR(-EINVAL);
 	}
 
-	return tok->type;
-}
-
-/*
- * This function returns 0 in case of invalid token. One should call
- * token_type() first to find out if the token is valid or not.
- */
-static enum opal_token response_get_token(const struct parsed_resp *resp,
-					  int n)
-{
-	const struct opal_resp_tok *tok;
-
-	if (n >= resp->num) {
-		pr_err("Token number doesn't exist: %d, resp: %d\n",
-		       n, resp->num);
-		return 0;
-	}
-
-	tok = &resp->toks[n];
-	if (tok->len == 0) {
-		pr_err("Token length must be non-zero\n");
-		return 0;
-	}
-
-	return tok->pos[0];
+	return tok;
 }
 
 static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
@@ -851,20 +828,32 @@ static u64 response_get_u64(const struct parsed_resp *resp, int n)
 	return resp->toks[n].stored.u;
 }
 
+static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
+{
+	if (IS_ERR_OR_NULL(token) ||
+	    token->type != OPAL_DTA_TOKENID_TOKEN ||
+	    token->pos[0] != match)
+		return false;
+	return true;
+}
+
 static u8 response_status(const struct parsed_resp *resp)
 {
-	if (token_type(resp, 0) == OPAL_DTA_TOKENID_TOKEN &&
-	    response_get_token(resp, 0) == OPAL_ENDOFSESSION) {
+	const struct opal_resp_tok *tok;
+
+	tok = response_get_token(resp, 0);
+	if (response_token_matches(tok, OPAL_ENDOFSESSION))
 		return 0;
-	}
 
 	if (resp->num < 5)
 		return DTAERROR_NO_METHOD_STATUS;
 
-	if (token_type(resp, resp->num - 1) != OPAL_DTA_TOKENID_TOKEN ||
-	    token_type(resp, resp->num - 5) != OPAL_DTA_TOKENID_TOKEN ||
-	    response_get_token(resp, resp->num - 1) != OPAL_ENDLIST ||
-	    response_get_token(resp, resp->num - 5) != OPAL_STARTLIST)
+	tok = response_get_token(resp, resp->num - 5);
+	if (!response_token_matches(tok, OPAL_STARTLIST))
+		return DTAERROR_NO_METHOD_STATUS;
+
+	tok = response_get_token(resp, resp->num - 1);
+	if (!response_token_matches(tok, OPAL_ENDLIST))
 		return DTAERROR_NO_METHOD_STATUS;
 
 	return response_get_u64(resp, resp->num - 4);
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1581679

On Wed, Feb 15, 2017 at 12:42:07PM -0700, Jon Derrick wrote:
> Add helper which verifies the response token is valid and matches the
> expected value. Merges token_type and response_get_token.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  block/sed-opal.c | 61 +++++++++++++++++++++++---------------------------------
>  1 file changed, 25 insertions(+), 36 deletions(-)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index 77623ad..d6dd604 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -591,48 +591,25 @@ static int cmd_finalize(struct opal_dev *cmd, u32 hsn, u32 tsn)
>  	return 0;
>  }
>  
> -static enum opal_response_token token_type(const struct parsed_resp *resp,
> -					   int n)
> +static const struct opal_resp_tok *response_get_token(
> +				const struct parsed_resp *resp,
> +				int n)
>  {
>  	const struct opal_resp_tok *tok;
>  
>  	if (n >= resp->num) {
>  		pr_err("Token number doesn't exist: %d, resp: %d\n",
>  		       n, resp->num);
> -		return OPAL_DTA_TOKENID_INVALID;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	tok = &resp->toks[n];
>  	if (tok->len == 0) {
>  		pr_err("Token length must be non-zero\n");
> -		return OPAL_DTA_TOKENID_INVALID;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
> -	return tok->type;
> -}
> -
> -/*
> - * This function returns 0 in case of invalid token. One should call
> - * token_type() first to find out if the token is valid or not.
> - */
> -static enum opal_token response_get_token(const struct parsed_resp *resp,
> -					  int n)
> -{
> -	const struct opal_resp_tok *tok;
> -
> -	if (n >= resp->num) {
> -		pr_err("Token number doesn't exist: %d, resp: %d\n",
> -		       n, resp->num);
> -		return 0;
> -	}
> -
> -	tok = &resp->toks[n];
> -	if (tok->len == 0) {
> -		pr_err("Token length must be non-zero\n");
> -		return 0;
> -	}
> -
> -	return tok->pos[0];
> +	return tok;
>  }
>  
>  static ssize_t response_parse_tiny(struct opal_resp_tok *tok,
> @@ -851,20 +828,32 @@ static u64 response_get_u64(const struct parsed_resp *resp, int n)
>  	return resp->toks[n].stored.u;
>  }
>  
> +static bool response_token_matches(const struct opal_resp_tok *token, u8 match)
> +{
> +	if (IS_ERR_OR_NULL(token) ||
> +	    token->type != OPAL_DTA_TOKENID_TOKEN ||
> +	    token->pos[0] != match)
> +		return false;
> +	return true;
> +}
> +

This is sorta pedantic but from my reading of the code token can never be null. It will
either be a valid pointer or ERR_PTR(-EINVAL), but never null.
So maybe change this to IS_ERR(token) etc. Other than that little nit:

Reviewed-by: Scott Bauer <scott.bauer@intel.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web