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
