From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47761) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y7A6D-0000Gv-IX for qemu-devel@nongnu.org; Fri, 02 Jan 2015 16:49:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y7A6C-0004k6-Aa for qemu-devel@nongnu.org; Fri, 02 Jan 2015 16:49:37 -0500 Received: from mail.lekensteyn.nl ([2a02:2308::360:1:25]:35068) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y7A6C-0004jp-0P for qemu-devel@nongnu.org; Fri, 02 Jan 2015 16:49:36 -0500 From: Peter Wu Date: Fri, 02 Jan 2015 22:49:30 +0100 Message-ID: <39451895.Sj3lz7Pyu8@al> In-Reply-To: <54A6EA59.3000106@redhat.com> References: <1419692504-29373-1-git-send-email-peter@lekensteyn.nl> <7AE70E8E-E3F8-4AB2-B2E0-61B0A1FD3623@lekensteyn.nl> <54A6EA59.3000106@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Subject: Re: [Qemu-devel] [PATCH 00/10] block/dmg: (compatibility) fixes and bzip2 support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org On Friday 02 January 2015 13:58:33 John Snow wrote: >=20 > On 01/02/2015 01:46 PM, Peter Wu wrote: > > FYI, I plan to make some more changes: > > > > - do not require offset =E2=89=A0 0 for resource fork and XML offse= ts. > > Technically it is allowed, do you agree on this change? >=20 > If you have seen this in the wild, I definitely agree. If you haven't= , I=20 > am not against the change, but there's likely no hurry to include it = in=20 > this series if the changes are not simple. It would involve only a removal of "rsrc_fork_offset !=3D 0 && " in pat= ch 3 and "plist_xml_offset !=3D 0 && " in patch 5. I have not seen it in t= he real world, only when trying to construct a dmg file by hand for testin= g purposes. The change is simple and can be squashed in the patch. It makes sense since previously only the offset was checked. Now the length is checked instead. Before: /* read offset */ ret =3D read_uint64(bs, offset, &info_begin); if (ret < 0) { goto fail; } else if (info_begin =3D=3D 0) { /* assume invalid file when offset is zero */ ret =3D -EINVAL; goto fail; }=20 After (in current patch series): /* offset of resource fork (RsrcForkOffset) */ ret =3D read_uint64(bs, offset + 0x28, &rsrc_fork_offset); if (ret < 0) { goto fail; } ret =3D read_uint64(bs, offset + 0x30, &rsrc_fork_length); if (ret < 0) { goto fail; } // ... if (rsrc_fork_offset !=3D 0 && rsrc_fork_length !=3D 0) { ret =3D dmg_read_resource_fork(bs, &ds, =20 In the current patch series both the offset and lengths are checked, bu= t it is sufficient to look at just the length. Kind regards, Peter > > - improve offset checking > > https://git.lekensteyn.nl/peter/qemu/commit/?h=3Dblock-dmg-2.3&id=3D= 41fd83773361923f668f54796ff563660b77e96c > > (squash with the existing length checking patch) > > > > - (not part of this series, but for future consideration) read > > errors currently return 1 (EPERM). EIO or EINVAL would probably a > > better choice depending on the error type. > > > > Other than that, the patches should be ready for review. Thank you > > in advance. > > > > Kind regards, > > Peter > > https://lekensteyn.nl > > (pardon my brevity, top-posting and formatting, sent from my phone)= > > > > > > On January 2, 2015 5:31:33 PM CET, John Snow wro= te: > >> > >> > >> On 01/02/2015 09:14 AM, Stefan Hajnoczi wrote: > >>> On Sat, Dec 27, 2014 at 04:01:34PM +0100, Peter Wu wrote: > >>>> These series improve QEMU support for DMG image files: > >>> > >>> Hi, > >>> Thanks for this patch series. Kevin and I consider patches for > >> merging > >>> after they have a Reviewed-by: from at least 1 other QEMU > >> contributor. > >>> > >>> I have CCed John Snow. > >>> > >>> John: If you are busy, please CC someone else or let us know so t= his > >>> series can get reviewed. > >>> > >>> Stefan > >>> > >> > >> Just recomposing myself post-vacation, I will start looking this o= ver > >> today. > >> > >> --John