From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1PHf4y-0004OV-SH for mharc-grub-devel@gnu.org; Sun, 14 Nov 2010 11:05:20 -0500 Received: from [140.186.70.92] (port=55092 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PHf4u-0004OE-Lb for grub-devel@gnu.org; Sun, 14 Nov 2010 11:05:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PHf4q-0004pv-Hy for grub-devel@gnu.org; Sun, 14 Nov 2010 11:05:16 -0500 Received: from mail-bw0-f41.google.com ([209.85.214.41]:61187) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PHf4q-0004pc-8d for grub-devel@gnu.org; Sun, 14 Nov 2010 11:05:12 -0500 Received: by bwz16 with SMTP id 16so4656210bwz.0 for ; Sun, 14 Nov 2010 08:05:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:subject:references:in-reply-to :x-enigmail-version:content-type; bh=lAlgCxL6tmlTz8XrXRugRJzW0WziUV7d0D3iYjWn6VI=; b=emznbf1WJYyEdKaqogvK90EHCs13bB1rL5IN61+d5RWANgdRlgdpejLdF/bbiJjSrs RR0fjGta4qqHvIp+uc+ritRXH9w9P8RkNcRIBSa6aDdCzm2Z3egHwe6mawp6F4qXUhEl s/fWXWu0BVYC6ZHTbEtjBF28uxZuTYl2Zgzz0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:x-enigmail-version:content-type; b=HdQQihPVRpX+oyw7epSygP0oR6ha41de/vfBkmYVn9uvPoszd4QoNKRHV0zn907LP/ FRK4KYo0N1YOx1Ezskt4kiu+GyGRCxYQjgJNX/bJbES8eMmoDyu4jHz2vt10SKCwu4+d M1WBIP9TWvUR3mjeBloHFdvEsF2qcbq97tstU= Received: by 10.204.69.145 with SMTP id z17mr438224bki.32.1289750710735; Sun, 14 Nov 2010 08:05:10 -0800 (PST) Received: from debian.bg45.phnet (gprs45.swisscom-mobile.ch [193.247.250.45]) by mx.google.com with ESMTPS id 4sm2546715bki.13.2010.11.14.08.05.06 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sun, 14 Nov 2010 08:05:08 -0800 (PST) Message-ID: <4CE008AD.7070303@gmail.com> Date: Sun, 14 Nov 2010 17:05:01 +0100 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101030 Icedove/3.0.10 MIME-Version: 1.0 To: grub-devel@gnu.org References: <4CD5B3B7.1070006@gmail.com> In-Reply-To: X-Enigmail-Version: 1.0.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enigC75251441609B78B7D23A725" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: UDF filesystem fixes for grub X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Nov 2010 16:05:18 -0000 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigC75251441609B78B7D23A725 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable All patches committed Thanks On 11/11/2010 09:56 AM, Giuseppe Caizzone wrote: > 2010/11/6 Vladimir '=CF=86-coder/phcoder' Serbinenko : > =20 >> On 11/02/2010 02:10 PM, Giuseppe Caizzone wrote: >> =20 >>> Hello, >>> >>> I've made 4 small patches that improve the UDF filesystem support in >>> grub. With these changes, I've been able to read any regular file in >>> UDF filesystems created by both Linux and Windows on both optical >>> storage and USB sticks. >>> [...] >>> 1/4) Support for large (> 2GiB) files >>> [...] >>> The patch just changes an int and a grub_uint32_t to be grub_off_t. >>> >>> >>> =20 >> Good patch. Can you write the ChangeLog entry for it? >> =20 > OK - I've never written one, so I tried to mimic grub's. > > Support reading files larger than 2 GiB. > > * grub-core/fs/udf.c (grub_udf_iterate_dir): change type of variable > "offset" from grub_uint32_t to grub_off_t. > (grub_udf_read_file): change type of parameter "pos" from int to > grub_off_t. > > =20 >>> 2/4) Support for deleted files >>> [...] >>> The patch just skips directory entries with the "deleted" bit set, in= >>> grub_udf_iterate_dir(). >>> >>> >>> =20 >> This one is good too. Changelog entry? >> =20 > Properly handle deleted files. > > * grub-core/fs/udf.c (grub_udf_iterate_dir): Skip directory entries > whose "characteristics" field has the bit GRUB_UDF_FID_CHAR_DELETED > set. > > =20 >>> 3/4) Support for a generic block size >>> [...] >>> The patch repeats the superblock search in grub_udf_mount() for 512, >>> 1024, 2048 and 4096 block sizes, stopping if it finds a valid one AND= >>> the sector offset recorded in that superblock matches the detected >>> superblock offset. So for instance it won't misdetect a superblock >>> located at sector 256 with blocksize 2048 for a superblock located at= >>> sector 512 with blocksize 1024. >>> The log2(blocksize/512) is then stored in a new field called "lbshift= " >>> in struct grub_udf_data, which gets used instead of the previous >>> constant GRUB_UDF_LOG2_BLKSZ, while the constant GRUB_UDF_BLKSZ gets >>> replaced with the lvd.bsize field already present in struct >>> grub_udf_data. >>> >>> >>> =20 >> + lbshift =3D 0; >> + block =3D 0; >> + while (lbshift <=3D 3) >> + { >> For loop is way more appropriate than a while loop >> =20 > Changed it into a for loop. > > =20 >> + while (lbshift <=3D 3) >> + { >> + sblklist =3D sblocklist; >> + while (*sblklist) >> + { >> Same here. >> =20 > Changed that too. > > I also made another change since the previous version of the patch: I > moved the VRS check after the AVDP search, because it needs to know > the logical block size. I originally thought that it wasn't necessary, > because the VRS is made up of records with a fixed size of 2048 bytes, > but the catch is that the standard says that each record must start on > a new sector, so if block size is > 2048, one has to skip more bytes > than 2048 to get to the next record. Tested it with mkudffs because I > have no medium with an actual block size of 4096. > > =20 >> ChangeLog entry? >> =20 > Add generic logical block size support. > > * grub-core/fs/udf.c (GRUB_UDF_LOG2_BLKSIZE): Removed macro. > (GRUB_UDF_BLKSZ): Removed macro. > (struct grub_udf_data): New field "lbshift" to hold the logical block > size of the file system in log2 format. > (grub_udf_read_icb): Replace usage of macro GRUB_UDF_LOG2_BLKSZ with > field lbshift from struct grub_udf_data. > (grub_udf_read_file): Likewise. > (grub_udf_read_block): Replace usage of macro GRUB_UDF_BLKSZ with > field "lvd.bsize" from struct grub_udf_data. > Replace division with right shift. > (sblocklist): Change type to unsigned. > (grub_udf_mount): Change type of "sblklist" to unsigned. > New variable "vblock" to be used during VRS recognition. > New variable "lbshift" to hold the detected logical block size. > Move AVDP search before VRS recognition, because the latter requires > knowledge of the logical block size, which is detected during the > former. > Detect and validate logical block size during AVDP search, adding > support for block sizes 512, 1024 and 4096. > Make VRS recognition independent of block size. > Replace usages of macro GRUB_UDF_LOG2_BLKSZ with variable lbshift. > > =20 >>> 4/4) Support for allocation descriptor extensions >>> [...] >>> The patch checks, in grub_udf_read_block(), whether the "allocation >>> descriptor type", previously ignored, is 3, and in this case, it load= s >>> the referenced block, checks that it's a valid allocation extension >>> descriptor, and sets the "ad" pointer (and the "len" limit) to >>> continue the iteration from the allocation descriptors contained in >>> that block. >>> >>> This last one is the only patch which actually contains a proper new >>> block of code, and it allocates a big structure on the stack (it's a >>> sector, so it's up to 4K big): I don't know if this is OK, or if >>> perhaps I should use grub_malloc() instead. Also, the patch depends o= n >>> the previous "generic block size" patch. >>> >>> >>> =20 >> + char buf[U32 (node->data->lvd.bsize)]; >> Will segfault if bsize is too big. You need to allocate on heap >> =20 > Done. > > =20 >> - else >> + else if (U16 (node->fe.tag.tag_ident =3D=3D GRUB_UDF_TAG_IDENT_EFE)= ) >> { >> Parenthesis are wrong. >> =20 > While I was at it, I changed the 'if-else if' to a switch. > > =20 >> + grub_disk_addr_t sec =3D grub_udf_get_block(node->data, >> + node->part_ref= , >> + ad->position);= >> ad->position isn't byte-swapped. >> =20 > grub_udf_get_block() byte-swaps it itself, so I must not byte-swap it b= efore. > > =20 >> ChangeLog entry? >> =20 > Add support for allocation extent descriptors, needed on fragmented > volumes. > > * grub-core/fs/udf.c (grub_udf_aed): New struct. > (grub_udf_read_block): Change type of variable "len" to grub_ssize_t. > Add sanity check for file entry type. > Replace divisions with right shifts. > Check the upper bits of the allocation descriptor's length and honour > them by loading an allocation extent descriptor if they indicate so. > Change all failure return paths to free the allocation extent > descriptor if it was allocated. > > I'm attaching the full patch set (including the unchanged ones for > convenience), and also the change log entries in attachment form in > case gmail tampers with whitespace. > > Thank you, > =20 > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > =20 --=20 Regards Vladimir '=CF=86-coder/phcoder' Serbinenko --------------enigC75251441609B78B7D23A725 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREKAAYFAkzgCK0ACgkQNak7dOguQglpbAEAlU5HI9JhXKScl2eP9i+ZkN+M vSafaZKJUm54EBmPfFMA/RdH5kJxYSxv3sp1a42ks6CbuH9EcNQ1zQvosBuoJtj/ =xcgw -----END PGP SIGNATURE----- --------------enigC75251441609B78B7D23A725--