From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44655) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V5Nou-00019T-L8 for qemu-devel@nongnu.org; Fri, 02 Aug 2013 18:27:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V5Noo-00042w-V1 for qemu-devel@nongnu.org; Fri, 02 Aug 2013 18:27:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2995) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V5Noo-00042L-Mh for qemu-devel@nongnu.org; Fri, 02 Aug 2013 18:27:30 -0400 Message-ID: <51FC324D.2070105@redhat.com> Date: Fri, 02 Aug 2013 16:27:25 -0600 From: Eric Blake MIME-Version: 1.0 References: <1375434137-4452-1-git-send-email-gesaint@linux.vnet.ibm.com> <1375434137-4452-4-git-send-email-gesaint@linux.vnet.ibm.com> In-Reply-To: <1375434137-4452-4-git-send-email-gesaint@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="V5BieOPT5S2U93CioTt6QxTOXCbrC12X6" Subject: Re: [Qemu-devel] [PATCH V5 3/6] block: Add WIN32 platform support for backing_file_loop_check() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xu Wang Cc: kwolf@redhat.com, famz@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, wdongxu@linux.vnet.ibm.com, Xu Wang , xiawenc@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --V5BieOPT5S2U93CioTt6QxTOXCbrC12X6 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08/02/2013 03:02 AM, Xu Wang wrote: > From: Xu Wang >=20 > Method of get_inode is different between Linux and WIN32 plateform. s/plateform/platform/g (3 instances in the commit message) > This patch added inode caculate method on Windows plateform so that s/added/adds an/ s/caculate/calculation/ > backing file check could work on Windows plateform. >=20 > Signed-off-by: Xu Wang > --- > block.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++---- > 1 file changed, 148 insertions(+), 8 deletions(-) >=20 > } > =20 > +#ifdef _WIN32 > +static int get_lnk_target_file(const char *lnk_file, char *filepath) > +{ > + unsigned int flag, offset; > + unsigned int sflag; > + char uch; > + int i =3D 0; > + > + FILE *fd =3D fopen(lnk_file, "rb"); > + if (!fd) { > + error_report("Open file %s failed.", lnk_file); Error messages should not include '.'; also, when it is due to a system call failure, you should include strerror() information explaining the failure. > + return -1; > + } > + > + /* Check if the file is shortcuts by reading first 4 bytes if it's= 0x4c */ > + if (fread(&flag, 4, 1, fd) !=3D 1) { > + error_report("Read 0x4c field of %s failed.", lnk_file); I don't know WIN32 programming well enough to know if this really how you should be checking for infinite loops. But how is this supposed to work? In POSIX, fopen() of a symlink opens its destination; to read a symlink's contents, you have to use readlink() (that is, the API used to read file contents is NOT the API used to chase link resolution). This whole function feels like a horrible hack, unlikely to do what you meant.= > + > +static long get_inode(const char *filename) Again, 'long' and 'ino_t' are not necessarily compatible types. Use ino_t. And again, 'ino_t' alone does not uniquely designate a file; it is the combination of device and inode numbers together that give uniqueness. > +{ > + #ifdef _WIN32 We usually anchor # in the first column. > + char pbuf[PATH_MAX + 1], *p; > + long inode; > + struct stat sbuf; > + char path[PATH_MAX + 1]; > + int len; > + > + /* If filename contains .lnk, it's a shortcuts. Target file > + * need to be parsed. How does the rest of the qemu code base handle .lnk files? Are we trying to dereference the target file automatically? If so, is there already code that does that, which we can reuse here? It just seems awkward that you are implementing this from scratch - either we support reading through windows links (and should reuse that code) or we don't (and hence it doesn't matter if the user passes us a .lnk file, we aren't treating it any different than any other data file). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --V5BieOPT5S2U93CioTt6QxTOXCbrC12X6 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.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJR/DJNAAoJEKeha0olJ0Nqs6sH/1/9OjAwvZx2C9Ax0xcQWg5p I8ycpqrOH5fLSrLicPp5wLXW8nH5DWAZSux6vrQ+c74QIRsK+nAbs1d1s4USF0lV EerOw6Eoh+syMWnunRRMlLK9VGdidgdv5FWAkxg1IslS5kqRcchqGHWOH9a+tUKm wU4flCM+dhOooqnDkT8Hzyc3ZtbvfdXfIGSAO+9+aoAMA5pcsqOTcQ6SZTjNRMY/ fdIDHKklmMoFH3m/S26H5OCj7nY/I1GaaDvI6LniADCO6t5DtVI5dcZ2/2W+lnIT yuGiyJjNbR7D+IGbFEp5HDRqz2jKGAzV1Zx+sE60nIt+9tRYM1RlhdBU6BzPVMc= =D3vd -----END PGP SIGNATURE----- --V5BieOPT5S2U93CioTt6QxTOXCbrC12X6--