linux-kernel - Re: [PATCH 24/29] Staging: yaffs2: yaffs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.00.1011012008351.12889@swampdragon.chaosbits.net>
Date:	Mon, 1 Nov 2010 20:16:19 +0100 (CET)
From:	Jesper Juhl <jj@...osbits.net>
To:	Tracey Dent <tdent48227@...il.com>
cc:	greg@...ah.com, manningc2@...rix.gen.nz,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 24/29] Staging: yaffs2: yaffs_verify: Add files

On Mon, 1 Nov 2010, Tracey Dent wrote:

> Adding files to yaffs2 directory.
> 
> Signed-off-by: Tracey Dent <tdent48227@...il.com>
> ---
>  drivers/staging/yaffs2/yaffs_verify.c |  631 +++++++++++++++++++++++++++++++++
>  drivers/staging/yaffs2/yaffs_verify.h |   41 +++
>  2 files changed, 672 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/staging/yaffs2/yaffs_verify.c
>  create mode 100644 drivers/staging/yaffs2/yaffs_verify.h
> 
> diff --git a/drivers/staging/yaffs2/yaffs_verify.c b/drivers/staging/yaffs2/yaffs_verify.c
> new file mode 100644
> index 0000000..89e730b
> --- /dev/null
> +++ b/drivers/staging/yaffs2/yaffs_verify.c
...
> +#if 0

Why are you trying to merge new code that's under '#if 0' (and not just 
here, but elsewhere as well)?
Either it shouldn't be here, in which case it should just be omitted from 
the patch, or it needs to be here eventually in which case it should be 
fixed up so the '#if 0' can go away or simply just be added later when it 
has been fixed up.

 > +/* Not being used, but don't want to throw away yet 
*/ > +int yaffs_verify_tnode_worker(yaffs_obj_t *obj, yaffs_tnode_t *tn,
> +					__u32 level, int chunk_offset)
> +{
> +	int i;
> +	yaffs_dev_t *dev = obj->my_dev;
> +	int ok = 1;
> +
> +	if (tn) {
> +		if (level > 0) {
> +
> +			for (i = 0; i < YAFFS_NTNODES_INTERNAL && ok; i++) {
> +				if (tn->internal[i]) {
> +					ok = yaffs_verify_tnode_worker(obj,
> +							tn->internal[i],
> +							level - 1,
> +							(chunk_offset<<YAFFS_TNODES_INTERNAL_BITS) + i);
> +				}
> +			}
> +		} else if (level == 0) {
> +			yaffs_ext_tags tags;
> +			__u32 obj_id = obj->obj_id;
> +
> +			chunk_offset <<=  YAFFS_TNODES_LEVEL0_BITS;
> +
> +			for (i = 0; i < YAFFS_NTNODES_LEVEL0; i++) {
> +				__u32 the_chunk = yaffs_get_group_base(dev, tn, i);
> +
> +				if (the_chunk > 0) {
> +					/* T(~0,(TSTR("verifying (%d:%d) %d"TENDSTR),tags.obj_id,tags.chunk_id,the_chunk)); */
> +					yaffs_rd_chunk_tags_nand(dev, the_chunk, NULL, &tags);
> +					if (tags.obj_id != obj_id || tags.chunk_id != chunk_offset) {
> +						T(~0, (TSTR("Object %d chunk_id %d NAND mismatch chunk %d tags (%d:%d)"TENDSTR),
> +							obj_id, chunk_offset, the_chunk,
> +							tags.obj_id, tags.chunk_id));
> +					}
> +				}
> +				chunk_offset++;
> +			}
> +		}
> +	}
> +
> +	return ok;

Here I think it would be nice to reduce the indentation level a bit by by 
doing something like:

int yaffs_verify_tnode_worker(yaffs_obj_t *obj, yaffs_tnode_t *tn,
                                     __u32 level, int chunk_offset)
{
     int i;
     yaffs_dev_t *dev = obj->my_dev;
     int ok = 1;
                     
     if (!tn)
          goto out; /* or simply  return ok;  */

     ... all the stuff currently inside the if() ...

 out:
     return ok;
}


-- 
Jesper Juhl <jj@...osbits.net>             http://www.chaosbits.net/
Plain text mails only, please      http://www.expita.com/nomime.html
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/