Daryl, Community I've found another logical error inside IIO_Write(). It's related to flush of OutBuf with wide characters. Currently the number of flushed characters from OutBuf is correct only for a conditional branch with wide character device but may be incorrect for non-latin symbols and multi-byte device. For multi-byte character device more characters are flushed than it should be. So please consider more appropriate patch which fixes improper flush and guarantee return of a proper number of bytes sent to any device type. Happy New Year!
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Alexey Vegner <[email protected]> --- The patch for https://github.com/tianocore/edk2/blob/master/StdLib/LibC/Uefi/InteractiveIO/IIO.c: diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIO.c b/StdLib/LibC/Uefi/InteractiveIO/IIO.c index 65b61d9..7375c74 100644 --- a/StdLib/LibC/Uefi/InteractiveIO/IIO.c +++ b/StdLib/LibC/Uefi/InteractiveIO/IIO.c @@ -115,6 +115,7 @@ IIO_Write( mbstate_t *OutState; char *MbcsPtr; ssize_t NumWritten; + ssize_t NumToFlush; ssize_t NumProc; size_t CharLen; UINTN MaxColumn; @@ -148,7 +149,6 @@ IIO_Write( This->CurrentXY.Column = This->InitialXY.Column; This->CurrentXY.Row = This->InitialXY.Row; - NumWritten = 0; OutChar[0] = (wchar_t)buf[0]; while((OutChar[0] != 0) && (NumWritten < N)) { @@ -177,12 +177,18 @@ IIO_Write( if(filp->f_iflags & _S_IWTTY) { // Output device expects wide characters, Output what we have NumWritten = filp->f_ops->fo_write(filp, NULL, NumWritten, gMD->UString); + // Remember how many wide characters we need to flush + NumToFlush = NumWritten; + // The function gets buf with multi-byte string but writes to the device wide-character string. + // So we have to return how many bytes from buf have been sent. + gMD->UString[NumWritten] = '\0'; + NumWritten = (ssize_t)EstimateWtoM((const wchar_t *)gMD->UString, UNICODE_STRING_MAX * sizeof(wchar_t), NULL); } else { // Output device expects narrow characters, convert to MBCS MbcsPtr = (char *)gMD->UString2; // Determine the needed space - NumProc = (ssize_t)EstimateWtoM((const wchar_t *)gMD->UString, UNICODE_STRING_MAX * sizeof(wchar_t), &CharLen); + NumProc = (ssize_t)EstimateWtoM((const wchar_t *)gMD->UString, UNICODE_STRING_MAX * sizeof(wchar_t), NULL); // Now translate this into MBCS in Buffer NumWritten = wcstombs(MbcsPtr, (const wchar_t *)gMD->UString, NumProc); @@ -190,9 +196,12 @@ IIO_Write( // Send the MBCS buffer to Output NumWritten = filp->f_ops->fo_write(filp, NULL, NumWritten, MbcsPtr); + // Calculate how many wide characters we need to flush + MbcsPtr[NumWritten] = 0; + NumToFlush = CountMbcsChars(MbcsPtr); } - // Consume the translated characters - (void)OutBuf->Flush(OutBuf, NumWritten); + // Consume the translated wide characters + (void)OutBuf->Flush(OutBuf, NumToFlush); } else { if(This == NULL) { Best regards, Alexey Vegner Paragon Software Group Corporation http://www.paragon-software.com -----Original Message----- From: Daryl McDaniel [mailto:[email protected]] Sent: Tuesday, December 29, 2015 6:06 AM To: Alexey Vegner <[email protected]>; [email protected] Subject: RE: [edk2] [PATCH] StdLib: Return of proper number of bytes sent to the output device expecting wide characters Alexey, Thank you for bringing this to my attention and providing the patch and detailed analysis. There is a chance that I will not be able to review and commit this until next week. Please be patient. Happy Holidays, Daryl McDaniel > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Alexey Vegner > Sent: Monday, December 28, 2015 3:04 AM > To: [email protected] > Subject: [edk2] [PATCH] StdLib: Return of proper number of bytes sent > to the output device expecting wide characters > > When we try to print out some non-latin Unicode UTF-16 string by means > of *printf family functions some garbage is also printed out after the given > string. > The reason is IIO_Write() function > (StdLib/LibC/Uefi/InteractiveIO/IIO.c) returns invalid number of bytes > sent to the device expecting wide characters. > That function gets buf parameter (pointer to the MBC string to be > output) and returns number of bytes sent to the output device. But if > the device expects wide characters the function will return actually > number of sent wide characters instead of number of MBC characters > consumed from the buf. > It's obvious that number of MBC characters isn't equal to a number of > equivalent wide characters. It's greater. > This difference causes upper level functions to send more MBC > characters from beyond the bounds of the given string. > That's why garbage is printed out. Please look at the suggested patch. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Alexey Vegner <[email protected]> > --- > > The patch for > https://github.com/tianocore/edk2/blob/master/StdLib/LibC/Uefi/InteractiveIO/IIO. > c: > > diff --git a/StdLib/LibC/Uefi/InteractiveIO/IIO.c > b/StdLib/LibC/Uefi/InteractiveIO/IIO.c > index 65b61d9..362edd2 100644 > --- a/StdLib/LibC/Uefi/InteractiveIO/IIO.c > +++ b/StdLib/LibC/Uefi/InteractiveIO/IIO.c > @@ -177,6 +177,10 @@ IIO_Write( > if(filp->f_iflags & _S_IWTTY) { > // Output device expects wide characters, Output what we have > NumWritten = filp->f_ops->fo_write(filp, NULL, NumWritten, > gMD->UString); > + // The function gets buf with multi-byte string but writes to > + the device > wide-character string. > + // So we have to return how many bytes from buf have been sent. > + gMD->UString[NumWritten] = '\0'; > + NumWritten = (ssize_t)EstimateWtoM((const wchar_t > + *)gMD->UString, > UNICODE_STRING_MAX * sizeof(wchar_t), &CharLen); > } > else { > // Output device expects narrow characters, convert to MBCS > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
