Hello! Unless it is guaranteed an "int" will always have exactly 32-bits, using "INT32" instead of "int" does not guarantee the algorithm will work for machines where an "int" is not a 32-bit integer.
Thanks for your attention! Kind regards, Arthur. -----Original Message----- From: Long, Qin [mailto:[email protected]] Sent: terça-feira, 19 de janeiro de 2016 15:38 To: Laszlo Ersek; Burigo, Arthur Crippa; [email protected] Subject: RE: [edk2] [PATCH] CryptoPkg: Fix function qsort for non 32-bit machines Thanks for the detailed analysis. The fix makes sense to me. And yes, I also prefer to use INT32 as Laszlo's comment (since we don't consider ILP64 data model), to keep the consistent style in this function declaration. Best Regards & Thanks, LONG, Qin > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Wednesday, January 20, 2016 1:10 AM > To: Burigo, Arthur Crippa; [email protected] > Cc: Long, Qin > Subject: Re: [edk2] [PATCH] CryptoPkg: Fix function qsort for non > 32-bit machines > > On 01/19/16 16:13, Burigo, Arthur Crippa wrote: > > Although the function qsort receives as an argument a "compare" > > function which returns an "int", QuickSortWorker (the function used > > internally by qsort to do its job) receives as an argument a > > "CompareFunction" which returns an "INTN". In a 32-bit machine, > > "INTN" is defined as "INT32", which is defined as "int" and > > everything works well. However, when qsort is compiled for a 64-bit > > machine, "INTN" is defined as "INT64" and the return values of the > > compare functions become incompatible ("int" for qsort and "INT64" for > > QuickSortWorker), causing malfunction. > > > > For example, let's assume qsort is being compiled for a 64-bit machine. > > As stated before, the "compare" function will be returning an "int", > > and "CompareFunction" will be returning an "INT64". When, for > > example, the "compare" function (which was passed as an argument to > > qsort and, then, re-passed as an argument to QuickSortWorker) > > returns -1 (or 0xffffffff, in a 32-bit integer, its original return > > type) from inside a call to QuickSortWorker, its return value is > > interpreted as being an "INT64" > > value - which turns out to be 4294967295 (or 0x00000000ffffffff, in > > a 64-bit integer) -, making the function QuickSortWorker to behave > > unexpectedly. > > > > Note that this unexpected (or incorrect) conversion does not happen > > when casting an "INT32" to an "INT64" directly, but does happen when > > casting function types. > > > > The issue is fixed by changing the return type of SORT_COMPARE (the > > type of "CompareFunction", used by QuickSortWorker) from "INTN" to "int". > > This way, both qsort and QuickSortWorker use compatible definitions > > for their compare functions. > > > > Cc: Qin Long <[email protected]> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Acked-by: Paulo Alcantara Cavalcanti <[email protected]> > > Signed-off-by: Karyne Mayer <[email protected]> > > Signed-off-by: Rodrigo Dias Correa <[email protected]> > > Signed-off-by: Arthur Crippa Burigo <[email protected]> > > --- > > CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c > > b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c > > index fb446b6..f2c8987 100644 > > --- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c > > +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c > > @@ -22,7 +22,7 @@ FILE *stdin = NULL; FILE *stdout = NULL; > > > > typedef > > -INTN > > +int > > (*SORT_COMPARE)( > > IN VOID *Buffer1, > > IN VOID *Buffer2 > > > > INT32 is more idiomatic for edk2, I think. > > Thanks > Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
