LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 6/6] udf: super.c reorganization
Date: Wed, 6 Feb 2008 12:09:36 +0100	[thread overview]
Message-ID: <20080206110936.GC12547@duck.suse.cz> (raw)
In-Reply-To: <20080205193417.GC6332@joi>

On Tue 05-02-08 20:34:49, Marcin Slusarz wrote:
> On Tue, Feb 05, 2008 at 05:22:19PM +0100, Jan Kara wrote:
> >   Actually, the loop below would be even more readable it you did:
> > 
> >   if (map->s_partition_num == le16_to_cpu(p->partitionNumber))
> >     break;
> >   And do the work after we exit from the loop.
> > 
> > 
> > >  	for (i = 0; i < sbi->s_partitions; i++) {
> > > +		struct partitionHeaderDesc *phd;
> > > +
> > >  		map = &sbi->s_partmaps[i];
> > >  		udf_debug("Searching map: (%d == %d)\n",
> > >  			  map->s_partition_num,
> > >  			  le16_to_cpu(p->partitionNumber));
> > > -		if (map->s_partition_num ==
> > > -				le16_to_cpu(p->partitionNumber)) {
> > > -			map->s_partition_len =
> > > -				le32_to_cpu(p->partitionLength); /* blocks */
> > > -			map->s_partition_root =
> > > -				le32_to_cpu(p->partitionStartingLocation);
> > > -			if (p->accessType ==
> > > -					cpu_to_le32(PD_ACCESS_TYPE_READ_ONLY))
> > > -				map->s_partition_flags |=
> > > -						UDF_PART_FLAG_READ_ONLY;
> > > -			if (p->accessType ==
> > > -					cpu_to_le32(PD_ACCESS_TYPE_WRITE_ONCE))
> > > -				map->s_partition_flags |=
> > > -						UDF_PART_FLAG_WRITE_ONCE;
> > > -			if (p->accessType ==
> > > -					cpu_to_le32(PD_ACCESS_TYPE_REWRITABLE))
> > > +		if (map->s_partition_num !=
> > > +				le16_to_cpu(p->partitionNumber))
> > > +			continue;
> > > +
> > > +		map->s_partition_len =
> > > +			le32_to_cpu(p->partitionLength); /* blocks */
> > > +		map->s_partition_root =
> > > +			le32_to_cpu(p->partitionStartingLocation);
> > > +		if (p->accessType == cpu_to_le32(PD_ACCESS_TYPE_READ_ONLY))
> > > +			map->s_partition_flags |= UDF_PART_FLAG_READ_ONLY;
> > > +		if (p->accessType == cpu_to_le32(PD_ACCESS_TYPE_WRITE_ONCE))
> > > +			map->s_partition_flags |= UDF_PART_FLAG_WRITE_ONCE;
> > > +		if (p->accessType == cpu_to_le32(PD_ACCESS_TYPE_REWRITABLE))
> > > +			map->s_partition_flags |= UDF_PART_FLAG_REWRITABLE;
> > > +		if (p->accessType == cpu_to_le32(PD_ACCESS_TYPE_OVERWRITABLE))
> > > +			map->s_partition_flags |= UDF_PART_FLAG_OVERWRITABLE;
> > > +
> > > +		if (strcmp(p->partitionContents.ident,
> > > +				PD_PARTITION_CONTENTS_NSR02) &&
> > > +			strcmp(p->partitionContents.ident,
> > > +				PD_PARTITION_CONTENTS_NSR03))
> > > +			break;
> > > +
> > > +		phd = (struct partitionHeaderDesc *)
> > > +				(p->partitionContentsUse);
> > > +		if (phd->unallocSpaceTable.extLength) {
> > > +			kernel_lb_addr loc = {
> > > +				.logicalBlockNum = le32_to_cpu(
> > > +					phd->unallocSpaceTable.extPosition),
> > > +				.partitionReferenceNum = i,
> > > +			};
> > > +
> > > +			map->s_uspace.s_table =
> > > +				udf_iget(sb, loc);
> > > +			if (!map->s_uspace.s_table) {
> > > +				udf_debug("cannot load unallocSpaceTable "
> > > +					  "(part %d)\n", i);
> > > +				return 1;
> > > +			}
> > > +			map->s_partition_flags |=
> > > +				UDF_PART_FLAG_UNALLOC_TABLE;
> > > +			udf_debug("unallocSpaceTable (part %d) @ %ld\n",
> > > +					i, map->s_uspace.s_table->i_ino);
> > > +		}
> > > +		if (phd->unallocSpaceBitmap.extLength) {
> > > +			struct udf_bitmap *bitmap =
> > > +				udf_sb_alloc_bitmap(sb, i);
> > > +			map->s_uspace.s_bitmap = bitmap;
> > > +			if (bitmap != NULL) {
> > > +				bitmap->s_extLength = le32_to_cpu(
> > > +					phd->unallocSpaceBitmap.extLength);
> > > +				bitmap->s_extPosition = le32_to_cpu(
> > > +					phd->unallocSpaceBitmap.extPosition);
> > >  				map->s_partition_flags |=
> > > -						UDF_PART_FLAG_REWRITABLE;
> > > -			if (p->accessType ==
> > > -				    cpu_to_le32(PD_ACCESS_TYPE_OVERWRITABLE))
> > > +						UDF_PART_FLAG_UNALLOC_BITMAP;
> > > +				udf_debug("unallocSpaceBitmap (part %d) @ %d\n",
> > > +						i, bitmap->s_extPosition);
> > > +			}
> > > +		}
> > > +		if (phd->partitionIntegrityTable.extLength)
> > > +			udf_debug("partitionIntegrityTable (part %d)\n", i);
> > > +		if (phd->freedSpaceTable.extLength) {
> > > +			kernel_lb_addr loc = {
> > > +				.logicalBlockNum = le32_to_cpu(
> > > +					phd->freedSpaceTable.extPosition),
> > > +				.partitionReferenceNum = i,
> > > +			};
> > > +
> > > +			map->s_fspace.s_table =
> > > +				udf_iget(sb, loc);
> > > +			if (!map->s_fspace.s_table) {
> > > +				udf_debug("cannot load freedSpaceTable "
> > > +					  "(part %d)\n", i);
> > > +				return 1;
> > > +			}
> > > +			map->s_partition_flags |=
> > > +				UDF_PART_FLAG_FREED_TABLE;
> > > +			udf_debug("freedSpaceTable (part %d) @ %ld\n",
> > > +					i, map->s_fspace.s_table->i_ino);
> > > +		}
> > > +		if (phd->freedSpaceBitmap.extLength) {
> > > +			struct udf_bitmap *bitmap =
> > > +				udf_sb_alloc_bitmap(sb, i);
> > > +			map->s_fspace.s_bitmap = bitmap;
> > > +			if (bitmap != NULL) {
> > > +				bitmap->s_extLength = le32_to_cpu(
> > > +					phd->freedSpaceBitmap.extLength);
> > > +				bitmap->s_extPosition = le32_to_cpu(
> > > +					phd->freedSpaceBitmap.extPosition);
> > >  				map->s_partition_flags |=
> > > -						UDF_PART_FLAG_OVERWRITABLE;
> > > -
> > > -			if (!strcmp(p->partitionContents.ident,
> > > -				    PD_PARTITION_CONTENTS_NSR02) ||
> > > -			    !strcmp(p->partitionContents.ident,
> > > -				    PD_PARTITION_CONTENTS_NSR03)) {
> > > -				struct partitionHeaderDesc *phd;
> > > -
> > > -				phd = (struct partitionHeaderDesc *)
> > > -						(p->partitionContentsUse);
> > > -				if (phd->unallocSpaceTable.extLength) {
> > > -					kernel_lb_addr loc = {
> > > -						.logicalBlockNum = le32_to_cpu(phd->unallocSpaceTable.extPosition),
> > > -						.partitionReferenceNum = i,
> > > -					};
> > > -
> > > -					map->s_uspace.s_table =
> > > -						udf_iget(sb, loc);
> > > -					if (!map->s_uspace.s_table) {
> > > -						udf_debug("cannot load unallocSpaceTable (part %d)\n", i);
> > > -						return 1;
> > > -					}
> > > -					map->s_partition_flags |=
> > > -						UDF_PART_FLAG_UNALLOC_TABLE;
> > > -					udf_debug("unallocSpaceTable (part %d) @ %ld\n",
> > > -						  i, map->s_uspace.s_table->i_ino);
> > > -				}
> > > -				if (phd->unallocSpaceBitmap.extLength) {
> > > -					struct udf_bitmap *bitmap =
> > > -						udf_sb_alloc_bitmap(sb, i);
> > > -					map->s_uspace.s_bitmap = bitmap;
> > > -					if (bitmap != NULL) {
> > > -						bitmap->s_extLength =
> > > -							le32_to_cpu(phd->unallocSpaceBitmap.extLength);
> > > -						bitmap->s_extPosition =
> > > -							le32_to_cpu(phd->unallocSpaceBitmap.extPosition);
> > > -						map->s_partition_flags |= UDF_PART_FLAG_UNALLOC_BITMAP;
> > > -						udf_debug("unallocSpaceBitmap (part %d) @ %d\n",
> > > -							  i, bitmap->s_extPosition);
> > > -					}
> > > -				}
> > > -				if (phd->partitionIntegrityTable.extLength)
> > > -					udf_debug("partitionIntegrityTable (part %d)\n", i);
> > > -				if (phd->freedSpaceTable.extLength) {
> > > -					kernel_lb_addr loc = {
> > > -						.logicalBlockNum = le32_to_cpu(phd->freedSpaceTable.extPosition),
> > > -						.partitionReferenceNum = i,
> > > -					};
> > > -
> > > -					map->s_fspace.s_table =
> > > -						udf_iget(sb, loc);
> > > -					if (!map->s_fspace.s_table) {
> > > -						udf_debug("cannot load freedSpaceTable (part %d)\n", i);
> > > -						return 1;
> > > -					}
> > > -					map->s_partition_flags |=
> > > -						UDF_PART_FLAG_FREED_TABLE;
> > > -					udf_debug("freedSpaceTable (part %d) @ %ld\n",
> > > -						  i, map->s_fspace.s_table->i_ino);
> > > -				}
> > > -				if (phd->freedSpaceBitmap.extLength) {
> > > -					struct udf_bitmap *bitmap =
> > > -						udf_sb_alloc_bitmap(sb, i);
> > > -					map->s_fspace.s_bitmap = bitmap;
> > > -					if (bitmap != NULL) {
> > > -						bitmap->s_extLength =
> > > -							le32_to_cpu(phd->freedSpaceBitmap.extLength);
> > > -						bitmap->s_extPosition =
> > > -							le32_to_cpu(phd->freedSpaceBitmap.extPosition);
> > > -						map->s_partition_flags |= UDF_PART_FLAG_FREED_BITMAP;
> > > -						udf_debug("freedSpaceBitmap (part %d) @ %d\n",
> > > -							  i, bitmap->s_extPosition);
> > > -					}
> > > -				}
> > > +						UDF_PART_FLAG_FREED_BITMAP;
> > > +				udf_debug("freedSpaceBitmap (part %d) @ %d\n",
> > > +						i, bitmap->s_extPosition);
> > >  			}
> > > -			break;
> > >  		}
> > > +		break;
> > >  	}
> > >  	if (i == sbi->s_partitions)
> > >  		udf_debug("Partition (%d) not found in partition map\n",
> 
> Ok. I didn't notice that. But it won't be as trivial as the rest. Separate patch?
  You're reindenting the code anyway, so moving it to another place isn't a
big deal (doesn't make review any harder). So you can just merge it into
the same patch. Thanks.

										Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  reply	other threads:[~2008-02-06 11:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-03 18:36 [PATCH 0/6] udf: next round of cleanups marcin.slusarz
2008-02-03 18:36 ` [PATCH 1/6] udf: udf_get_block, inode_bmap - remove unneeded checks marcin.slusarz
2008-02-05 15:30   ` Jan Kara
2008-02-03 18:36 ` [PATCH 2/6] udf: create function for conversion from timestamp to timespec marcin.slusarz
2008-02-05 15:32   ` Jan Kara
2008-02-03 18:36 ` [PATCH 3/6] udf: convert udf_stamp_to_time to return struct timespec marcin.slusarz
2008-02-05 15:48   ` Jan Kara
2008-02-05 19:12     ` Marcin Slusarz
2008-02-06 13:25       ` Jan Kara
2008-02-03 18:36 ` [PATCH 4/6] udf: convert udf_stamp_to_time and udf_time_to_stamp to use timestamps marcin.slusarz
2008-02-05 15:59   ` Jan Kara
2008-02-05 19:21     ` Marcin Slusarz
2008-02-06 11:10       ` Jan Kara
2008-02-10 10:25         ` Marcin Slusarz
2008-02-03 18:36 ` [PATCH 5/6] udf: remove unneeded kernel_timestamp type marcin.slusarz
2008-02-05 16:01   ` Jan Kara
2008-02-10 10:29     ` Marcin Slusarz
2008-02-03 18:42 ` [PATCH 6/6] udf: super.c reorganization Marcin Slusarz
2008-02-05 16:22   ` Jan Kara
2008-02-05 19:34     ` Marcin Slusarz
2008-02-06 11:09       ` Jan Kara [this message]
2008-02-10 10:33         ` Marcin Slusarz

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=20080206110936.GC12547@duck.suse.cz \
    --to=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcin.slusarz@gmail.com \
    --subject='Re: [PATCH 6/6] udf: super.c reorganization' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).