All of lore.kernel.org
 help / color / mirror / Atom feed
From: Salah Triki <salah.triki@gmail.com>
To: Luis de Bethencourt <luisbg@osg.samsung.com>
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	viro@zeniv.linux.org.uk, salah.triki@acm.org
Subject: Re: [PATCH v3 1/2] befs: remove unused BEFS_BT_PARMATCH
Date: Tue, 9 Aug 2016 13:40:10 +0100	[thread overview]
Message-ID: <20160809124010.GB17313@pc> (raw)
In-Reply-To: <1470666081-5053-1-git-send-email-luisbg@osg.samsung.com>

On Mon, Aug 08, 2016 at 03:21:20PM +0100, Luis de Bethencourt wrote:
> befs_btree_find(), the only caller of befs_find_key(), only cares about if
> the return from that function is BEFS_BT_MATCH or not. It never uses the
> partial match given with BEFS_BT_PARMATCH. Make the overflow return clearer
> by having BEFS_BT_OVERFLOW instead of BEFS_BT_PARMATCH.
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
> v3: check for BEFS_BT_OVERFLOW instead of value == 0
> 
> 
> Hi,
> 
> Switching to using BEFS_BT_OVERFLOW. This makes logic of befs_find_key()
> clearer.
> 
> Thanks,
> Luis
> 
>  fs/befs/befs.h  |  2 +-
>  fs/befs/btree.c | 38 ++++++++++++++++----------------------
>  2 files changed, 17 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/befs/befs.h b/fs/befs/befs.h
> index c5c6cd1..a8ca7fc 100644
> --- a/fs/befs/befs.h
> +++ b/fs/befs/befs.h
> @@ -79,7 +79,7 @@ enum befs_err {
>  	BEFS_BT_END,
>  	BEFS_BT_EMPTY,
>  	BEFS_BT_MATCH,
> -	BEFS_BT_PARMATCH,
> +	BEFS_BT_OVERFLOW,
>  	BEFS_BT_NOT_FOUND
>  };
>  
> diff --git a/fs/befs/btree.c b/fs/befs/btree.c
> index 3f1a391..27b0336 100644
> --- a/fs/befs/btree.c
> +++ b/fs/befs/btree.c
> @@ -281,9 +281,9 @@ befs_btree_find(struct super_block *sb, const befs_data_stream *ds,
>  
>  	while (!befs_leafnode(this_node)) {
>  		res = befs_find_key(sb, this_node, key, &node_off);
> -		if (res == BEFS_BT_NOT_FOUND)
> +		/* if no key set, try the overflow node */
> +		if (res == BEFS_BT_OVERFLOW)
>  			node_off = this_node->head.overflow;
> -		/* if no match, go to overflow node */
>  		if (befs_bt_read_node(sb, ds, this_node, node_off) != BEFS_OK) {
>  			befs_error(sb, "befs_btree_find() failed to read "
>  				   "node at %llu", node_off);
> @@ -291,8 +291,7 @@ befs_btree_find(struct super_block *sb, const befs_data_stream *ds,
>  		}
>  	}
>  
> -	/* at the correct leaf node now */
> -
> +	/* at a leaf node now, check if it is correct */
>  	res = befs_find_key(sb, this_node, key, value);
>  
>  	brelse(this_node->bh);
> @@ -323,16 +322,12 @@ befs_btree_find(struct super_block *sb, const befs_data_stream *ds,
>   * @findkey: Keystring to search for
>   * @value: If key is found, the value stored with the key is put here
>   *
> - * finds exact match if one exists, and returns BEFS_BT_MATCH
> - * If no exact match, finds first key in node that is greater
> - * (alphabetically) than the search key and returns BEFS_BT_PARMATCH
> - * (for partial match, I guess). Can you think of something better to
> - * call it?
> - *
> - * If no key was a match or greater than the search key, return
> - * BEFS_BT_NOT_FOUND.
> + * Finds exact match if one exists, and returns BEFS_BT_MATCH.
> + * If there is no match and node's value array is too small for key, return
> + * BEFS_BT_OVERFLOW.
> + * If no match and node should countain this key, return BEFS_BT_NOT_FOUND.
>   *
> - * Use binary search instead of a linear.
> + * Uses binary search instead of a linear.
>   */
>  static int
>  befs_find_key(struct super_block *sb, struct befs_btree_node *node,
> @@ -355,9 +350,8 @@ befs_find_key(struct super_block *sb, struct befs_btree_node *node,
>  
>  	eq = befs_compare_strings(thiskey, keylen, findkey, findkey_len);
>  	if (eq < 0) {
> -		befs_error(sb, "<--- %s %s not found", __func__, findkey);
> -		befs_debug(sb, "<--- %s ERROR", __func__);
> -		return BEFS_BT_NOT_FOUND;
> +		befs_debug(sb, "<--- node can't contain %s", findkey);
> +		return BEFS_BT_OVERFLOW;
>  	}
>  
>  	valarray = befs_bt_valarray(node);
> @@ -385,12 +379,12 @@ befs_find_key(struct super_block *sb, struct befs_btree_node *node,
>  		else
>  			first = mid + 1;
>  	}
> -	if (eq < 0)
> -		*value = fs64_to_cpu(sb, valarray[mid + 1]);
> -	else
> -		*value = fs64_to_cpu(sb, valarray[mid]);
> -	befs_debug(sb, "<--- %s found %s at %d", __func__, thiskey, mid);
> -	return BEFS_BT_PARMATCH;
> +
> +	/* return an existing value so caller can arrive to a leaf node */
> +	*value = fs64_to_cpu(sb, valarray[mid]);
> +	befs_error(sb, "<--- %s %s not found", __func__, findkey);
> +	befs_debug(sb, "<--- %s ERROR", __func__);
> +	return BEFS_BT_NOT_FOUND;
>  }
>  
>  /**
> -- 
> 2.5.1
> 

Signed-off-by: Salah Triki <salah.triki@gmail.com>

Thanx :)
Salah

      parent reply	other threads:[~2016-08-09 12:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-05 12:41 [PATCH v2 1/2] befs: remove unused BEFS_BT_MATCH Luis de Bethencourt
2016-08-05 12:41 ` [PATCH v2 2/2] befs: fix typo in befs_find_key Luis de Bethencourt
2016-08-06 18:40   ` Salah Triki
2016-08-06 18:34 ` [PATCH v2 1/2] befs: remove unused BEFS_BT_MATCH Salah Triki
2016-08-06 18:44   ` Luis de Bethencourt
2016-08-08 14:21 ` [PATCH v3 1/2] befs: remove unused BEFS_BT_PARMATCH Luis de Bethencourt
2016-08-08 14:21   ` [PATCH v3 2/2] befs: fix typo in befs_find_key Luis de Bethencourt
2016-08-09 12:40   ` Salah Triki [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160809124010.GB17313@pc \
    --to=salah.triki@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luisbg@osg.samsung.com \
    --cc=salah.triki@acm.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.