From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.zytor.com (terminus.zytor.com [65.50.211.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B5A6E21D492C5 for ; Thu, 14 Sep 2017 23:47:59 -0700 (PDT) Received: from [192.168.15.2] (177.18.107.39.static.host.gvt.net.br [177.18.107.39] (may be forged)) (authenticated bits=0) by mail.zytor.com (8.15.2/8.15.2) with ESMTPSA id v8F6mbWn010174 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 14 Sep 2017 23:48:41 -0700 Date: Fri, 15 Sep 2017 03:48:31 -0300 User-Agent: K-9 Mail for Android In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B95A36E@SHSMSX151.ccr.corp.intel.com> References: <8c5a7ec1d6f908eabda755bb6d3bc9a28b14210e.1505277490.git.pcacjr@zytor.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B940874@shsmsx102.ccr.corp.intel.com> <37D67821-E356-4E48-B175-3107DEF070AC@zytor.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B95A36E@SHSMSX151.ccr.corp.intel.com> MIME-Version: 1.0 To: "Zeng, Star" , "Laszlo Ersek (lersek@redhat.com)" , "Ni, Ruiyu" , "Yao, Jiewen" CC: "Dong, Eric" , "Bi, Dandan" , "edk2-devel@lists.01.org" , "Kinney, Michael D" From: Paulo Alcantara Message-ID: <3DF79BE6-4CC0-4213-8A33-504C741288F4@zytor.com> Subject: Re: [PATCH] MdeModulePkg/UdfDxe: Remove negative comparison of unsigned number X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Sep 2017 06:47:59 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Star, On September 15, 2017 3:14:38 AM GMT-03:00, "Zeng, Star" wrote: >Hi, > >Based on recent issues about UDF since the code was pushed for >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014360=2Ehtm= l, I >want to raise some questions kindly=2E > >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014409=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014518=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014542=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014489=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014551=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014560=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014649=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014694=2Ehtm= l >https://lists=2E01=2Eorg/pipermail/edk2-devel/2017-September/014695=2Ehtm= l > >Is the code expected to be got upstream originally? (I may be a stupid >question since the code has been gotten upstream, I just want to double >confirm that as Paulo's reply below "I believed that it would never get >upstream"=2E) >Is the code ready to be in master? Should it be in Staging branch >first? Sorry if I wasn't clear enough, but when I sent UDF support to ML in ~3 ye= ars ago, nobody seemed to care and had no feedback since then, so I thought= it would never get accepted and then I gave up on trying to improve, clean= up, etc=2E -- I even started working write support for it but also gave up= =2E It would not make sense to work on it anymore=2E Right? I would not know to tell you in which branch it should go, but as the driv= er gives you the ability to access an UDF fs as it's supposed to, then, IMH= O, it'd be ready to go either way=2E=20 > > >Paulo, >Could you help do more evaluation to the code as you said "I *do* know >that the code really needs refactoring, documentation, etc"? I believe >you are most familiar with the code and know its quality=2E :) > > >BTW: More test seems need to be done before the code check in, for >example, build with various tool chains, ECC scan for coding style, >static tool scan, etc=2E That is what we(especially me) need to improve >in future when developing and reviewing=2E >Anyway, let's help keep improving UDF codes=2E Yep=2E We should refactor the code, fix coding style issues, improve docum= etation=2E Since I was the only one who tested it, it definitely needs a li= ttle more testing=2E With you help, we definitely can resolve all those TODO items=2E Thanks! Paulo > > > >Thanks, >Star >-----Original Message----- >From: Paulo Alcantara [mailto:pcacjr@zytor=2Ecom]=20 >Sent: Wednesday, September 13, 2017 1:47 PM >To: Zeng, Star ; edk2-devel@lists=2E01=2Eorg >Cc: Dong, Eric ; Ni, Ruiyu ; >Bi, Dandan >Subject: RE: [PATCH] MdeModulePkg/UdfDxe: Remove negative comparison of >unsigned number > > > >On September 13, 2017 2:08:54 AM GMT-03:00, "Zeng, Star" > wrote: >>I do not understand the context of the code=2E >>The change is good to fix the build failure, but I want to ask a=20 >>question before I gave Rb=2E :) >> >>Is it possible ReadFileInfo->FilePosition less than FilePosition? > >Nope=2E When doing my tests, I briefly looked at code how it's used and >also added an ASSERT() to make sure it is never lesser than >FilePosition=2E > >BTW, I *do* know that the code really needs refactoring, documentation, >etc=2E -- I didnt do that before because I believed that it would never >get upstream -- since its now -- I will look forward to that in my free >time=2E > >Its 2:46am here so I should get some sleep :-) Thanks! > >Paulo > >> >> >>Thanks, >>Star >>-----Original Message----- >>From: Paulo Alcantara [mailto:pcacjr@zytor=2Ecom] >>Sent: Wednesday, September 13, 2017 12:45 PM >>To: edk2-devel@lists=2E01=2Eorg >>Cc: Paulo Alcantara ; Zeng, Star=20 >>; Dong, Eric ; Ni, Rui= yu=20 >>; Bi, Dandan >>Subject: [PATCH] MdeModulePkg/UdfDxe: Remove negative comparison of=20 >>unsigned number >> >>This patch gets rid of a negative comparison of an UINT64 type >(Offset)=20 >>as it'll never evaluate to true=2E >> >>Cc: Star Zeng >>Cc: Eric Dong >>Cc: Ruiyu Ni >>Cc: Dandan Bi >>Contributed-under: TianoCore Contribution Agreement 1=2E1 >>Reported-by: Star Zeng >>Signed-off-by: Paulo Alcantara >>--- >> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations=2Ec | 3 --- >> 1 file changed, 3 deletions(-) >> >>diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations=2Ec >>b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations=2Ec >>index 7286265373=2E=2E2039f80289 100644 >>--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations=2Ec >>+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations=2Ec >>@@ -1082,9 +1082,6 @@ ReadFile ( >>=20 >> if (FilePosition + ExtentLength > ReadFileInfo->FilePosition) >{ >> Offset =3D ReadFileInfo->FilePosition - FilePosition; >>- if (Offset < 0) { >>- Offset =3D -(Offset); >>- } >> } else { >> Offset =3D 0; >> } >>-- >>2=2E11=2E0 > >-- >Sent from my Android device with K-9 Mail=2E Please excuse my brevity=2E --=20 Sent from my Android device with K-9 Mail=2E Please excuse my brevity=2E