Subject: bug#16338: [PATCH 2/2] Fix filesystem detection on non 512 byte sectors

From: Phillip Susi <[email-masked]>
Date: Mon, 03 Mar 2014 19:36:43 -0500

On 03/03/2014 02:45 PM, Brian C. Lane wrote:
> Do our tests already cover all of these filesystems?

I think so but now that I look at this again, it looks like I forgot
to fix the tests.  Currently they skip on !512 byte sector size.  I'll
fix that.

> 
>> diff --git a/libparted/fs/ext2/interface.c
>> b/libparted/fs/ext2/interface.c index 97220b7..ecafb62 100644 ---
>> a/libparted/fs/ext2/interface.c +++
>> b/libparted/fs/ext2/interface.c @@ -33,10 +33,12 @@ struct
>> ext2_dev_handle*
>> ext2_make_dev_handle_from_parted_geometry(PedGeometry* g static
>> PedGeometry* _ext2_generic_probe (PedGeometry* geom, int
>> expect_ext_ver) { -	void *sb_v; -	if
>> (!ped_geometry_read_alloc(geom, &sb_v, 2, 2)) +	const int sectors
>> = (4096 + geom->dev->sector_size - 1) / +
>> geom->dev->sector_size; +	char *sb_v = alloca (sectors *
>> geom->dev->sector_size); +	if (!ped_geometry_read(geom, sb_v, 0,
>> sectors)) return NULL; -	struct ext2_super_block *sb = sb_v; +
>> struct ext2_super_block *sb = (struct ext2_super_block *)(sb_v +
>> 1024);
> 
> It would probably be more readable to just read_alloc the first 4
> blocks and then point *sb at +1024 inside that.
> 
> Also, I just realized that you are using alloca for these -- I'm
> not sure we want to switch to using something that has undefined
> behavior on failure.

Practically speaking it isn't undefined; if you really blow your stack
then you get a stack fault.  Also it seems a waste to read 4 sectors
when you only need 1.  I suppose I could add a comment there to
clarify the logic.