Re: ntfs: remove redundant assignments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Anton,
At some point in time, I wrote:
Index: 2.6-mm/fs/ntfs/super.c
> ===================================================================
> --- 2.6-mm.orig/fs/ntfs/super.c	2005-05-25 20:51:57.000000000 +0300
> +++ 2.6-mm/fs/ntfs/super.c	2005-05-25 20:54:02.000000000 +0300
> @@ -2283,7 +2283,7 @@
>  	sb->s_flags |= MS_RDONLY | MS_NOATIME | MS_NODIRATIME;
>  #endif /* ! NTFS_RW */
>  	/* Allocate a new ntfs_volume and place it in sb->s_fs_info. */
> -	sb->s_fs_info = kmalloc(sizeof(ntfs_volume), GFP_NOFS);
> +	sb->s_fs_info = kcalloc(1, sizeof(ntfs_volume), GFP_NOFS);
>  	vol = NTFS_SB(sb);
>  	if (!vol) {
>  		if (!silent)
> @@ -2292,28 +2292,9 @@
>  		return -ENOMEM;
>  	}
>  	/* Initialize ntfs_volume structure. */
> -	memset(vol, 0, sizeof(ntfs_volume));
> vol->sb = sb; The above is fine, thanks.
> -	vol->upcase = NULL;
> -	vol->attrdef = NULL;
> -	vol->mft_ino = NULL;
> -	vol->mftbmp_ino = NULL;
>  	init_rwsem(&vol->mftbmp_lock);
> -#ifdef NTFS_RW
> -	vol->mftmirr_ino = NULL;
> -	vol->logfile_ino = NULL;
> -#endif /* NTFS_RW */
> -	vol->lcnbmp_ino = NULL;
>  	init_rwsem(&vol->lcnbmp_lock);
> -	vol->vol_ino = NULL;
> -	vol->root_ino = NULL;
> -	vol->secure_ino = NULL;
> -	vol->extend_ino = NULL;
> -#ifdef NTFS_RW
> -	vol->quota_ino = NULL;
> -	vol->quota_q_ino = NULL;
> -#endif /* NTFS_RW */
> -	vol->nls_map = NULL;

On Wed, 2005-05-25 at 22:10 +0100, Anton Altaparmakov wrote:
This is not. memset(0) is not the same as = NULL IMO. I don't care if the compiler thinks it is the same. NULL does not have to be 0 so I prefer to initialize pointers explicitly to NULL. Even more so since this code is not performance critical at all so I prefer clarity here.

I kind of figured out you were doing it on purpose. The fact is, NULL is zero on _all_ Linux architectures. If it weren't, we'd have a lot of broken code. Let me play the devils advocate here: why do you memset() (now kcalloc()) in the first place?
At some point in time, I wrote:
> Index: 2.6-mm/fs/ntfs/index.c
> ===================================================================
> --- 2.6-mm.orig/fs/ntfs/index.c	2005-05-25 20:51:57.000000000 +0300
> +++ 2.6-mm/fs/ntfs/index.c	2005-05-25 21:07:38.000000000 +0300
> @@ -40,16 +40,8 @@
> > ictx = kmem_cache_alloc(ntfs_index_ctx_cache, SLAB_NOFS);
>  	if (ictx) {
> +		memset(ictx, 0, sizeof(*ictx));
>  		ictx->idx_ni = idx_ni;
> -		ictx->entry = NULL;
> -		ictx->data = NULL;
> -		ictx->data_len = 0;
> -		ictx->is_in_root = 0;
> -		ictx->ir = NULL;
> -		ictx->actx = NULL;
> -		ictx->base_ni = NULL;
> -		ictx->ia = NULL;
> -		ictx->page = NULL;

On Wed, 2005-05-25 at 22:10 +0100, Anton Altaparmakov wrote:
Again, as above, I prefer to have the explicit = NULL instead of a memset.

There's a simple reason why I don't like explicit assignments: it's way too easy to forget to initialize something. Pekka -
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux