I am not sure I like how the W_INSTRUMENT and R_INSTRUMENT look in the code but I also do not have a good alternate suggestion. It just looked odd when I was reading the patch.
Reviewed-by: Erik Bjorge <[email protected]> > -----Original Message----- > From: Daryl McDaniel [mailto:[email protected]] > Sent: Sunday, January 3, 2016 2:38 PM > To: 'Alexey Vegner' <[email protected]>; > [email protected]; Carsey, > Jaben <[email protected]>; Bjorge, Erik C <[email protected]> > Subject: [PATCH] StdLib: Fix IIO_Write() to return the number of bytes > consumed, not > characters output. > > StdLib: Fix IIO_Write() to return the number of bytes consumed, not > characters output. > > Depending upon termios settings, writing to a terminal device may result in > many more characters being output than were in the buffer provided to the > IIO_Write() function. > > IIO_Write() is supposed to return the number of BYTES written, not characters. > Since the provided buffer contains MBCS characters, there can be up to three > bytes per character. Due to the expansion that may occur, "BYTES written" > is interpreted to mean the number of BYTES consumed from the MBCS buffer > provided as a parameter to IIO_Write. > > These changes ensure that the correct number of characters are consumed from > the internal Output buffer and the correct number of BYTES consumed from the > buffer parameter are counted and returned. > > Update copyright. > Add debugging instrumentation to count unconsumed data in the Input and > Output buffers. > Improve comments for IIO_Write(). > Modify IIO_Write() to: > * Accurately count input bytes CONSUMED. > * Consume only as many expanded (cooked) characters from the output buffer > as were actually sent to the device. > * Purge only the number of CHARACTERS sent from the output buffer. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Daryl McDaniel <[email protected]> > > --- > diff U3 a/StdLib/LibC/Uefi/InteractiveIO/IIO.c > b/StdLib/LibC/Uefi/InteractiveIO/IIO.c > --- a/StdLib/LibC/Uefi/InteractiveIO/IIO.c Tue Dec 11 13:19:14 2012 > +++ b/StdLib/LibC/Uefi/InteractiveIO/IIO.c Sun Jan 03 09:35:14 2016 > @@ -3,6 +3,7 @@ > > The functions assume that isatty() is TRUE at the time they are called. > > + Copyright (c) 2016, Daryl McDaniel. All rights reserved.<BR> > Copyright (c) 2012, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials are licensed and made available > under the terms and conditions of the BSD License which accompanies this > @@ -26,6 +27,20 @@ > #include "IIOutilities.h" > #include "IIOechoCtrl.h" > > +// Instrumentation used for debugging > +#define IIO_C_DEBUG 0 ///< Set to 1 to enable instrumentation, 0 > to disable > + > +#if IIO_C_DEBUG > + static volatile size_t IIO_C_WRemainder = 0; ///< Characters in Out > buffer after > IIO_Write > + static volatile size_t IIO_C_RRemainder = 0; ///< Characters in In > buffer after > IIO_Read > + > + #define W_INSTRUMENT IIO_C_WRemainder = > + #define R_INSTRUMENT IIO_C_RRemainder = > +#else // ! IIO_C_DEBUG -- don't instrument code > + #define W_INSTRUMENT (void) > + #define R_INSTRUMENT (void) > +#endif // IIO_C_DEBUG > + > /** Read from an Interactive IO device. > > NOTE: If _S_IWTTY is set, the internal buffer contains WIDE characters. > @@ -82,24 +97,44 @@ > NumRead = wcstombs((char *)Buffer, (const wchar_t *)gMD->UString2, > XlateSz); > > // Consume the translated characters > - (void)This->InBuf->Flush(This->InBuf, Needed); > + (void) This->InBuf->Flush(This->InBuf, Needed); > } > else { > // Data in InBuf is narrow characters. Use verbatim. > NumRead = This->InBuf->Read(This->InBuf, Buffer, (INT32)BufferSize); > } > +#if IIO_C_DEBUG > + IIO_C_RRemainder = This->InBuf->Count(This->InBuf, AsElements); > +#endif // IIO_C_DEBUG > } > return NumRead; > } > > -/** Process characters from buffer buf and write them to the output device > +/** Handle write to a Terminal (Interactive) device. > + > + Processes characters from buffer buf and writes them to the Terminal > device > specified by filp. > > + The parameter buf points to a MBCS string to be output. This is processed > + and buffered one character at a time by IIO_WriteOne() which handles TAB > + expansion, NEWLINE to CARRIAGE_RETURN + NEWLINE expansion, as well as > + basic line editing functions. The number of characters actually written > to > + the output device will seldom equal the number of characters consumed > from > + buf. > + > + In this implementation, all of the special characters processed by > + IIO_WriteOne() are single-byte characters with values less than 128. > + (7-bit ASCII or the single-byte UTF-8 characters) > + > + Every byte that is not one of the recognized special characters is > passed, > + unchanged, to the Terminal device. > + > @param[in] filp Pointer to a file descriptor structure. > @param[in] buf Pointer to the MBCS string to be output. > @param[in] N Number of bytes in buf. > > - @retval >=0 Number of bytes sent to the output device. > + @retval >=0 Number of bytes consumed from buf and sent to the > + Terminal device. > **/ > static > ssize_t > @@ -114,16 +149,15 @@ > cFIFO *OutBuf; > mbstate_t *OutState; > char *MbcsPtr; > - ssize_t NumWritten; > + ssize_t NumConsumed; > ssize_t NumProc; > size_t CharLen; > UINTN MaxColumn; > UINTN MaxRow; > - wchar_t OutChar[2]; // Just in case we run into 4-byte MBCS > character > + wchar_t OutChar[2]; // Just in case we run into a 4-byte MBCS > character > int OutMode; > > - errno = 0; // indicate no error as default > - NumWritten = -1; > + NumConsumed = -1; > > /* Determine what the current screen size is. Also validates the output > device. */ > OutMode = IIO_GetOutputSize(filp->MyFD, &MaxColumn, &MaxRow); > @@ -148,51 +182,61 @@ > 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)) { > - CharLen = mbrtowc(OutChar, (const char *)&buf[NumWritten], MB_CUR_MAX, > OutState); > + NumConsumed = 0; > + OutChar[0] = (wchar_t)buf[0]; > + while((OutChar[0] != 0) && (NumConsumed < N)) { > + CharLen = mbrtowc(OutChar, (const char *)&buf[NumConsumed], > MB_CUR_MAX, OutState); > + if (CharLen < 0) { // Encoding Error > + OutChar[0] = BLOCKELEMENT_LIGHT_SHADE; > + CharLen = 1; // Consume a byte > + (void)mbrtowc(NULL, NULL, 1, OutState); // Re-Initialize the > conversion state > + } > NumProc = IIO_WriteOne(filp, OutBuf, OutChar[0]); > - if(NumProc > 0) { > + if(NumProc >= 0) { > // Successfully processed and buffered one character > - NumWritten += CharLen; // Index of start of next character > - } > - else if(NumProc == -1) { > - // Encoding Error > - (void)mbrtowc(NULL, NULL, 1, OutState); // Re-Initialize the > conversion state > - errno = EILSEQ; > - break; > + NumConsumed += CharLen; // Index of start of next character > } > else { > - // Last character was incomplete > - break; > + if (errno == ENOSPC) { > + // Not enough room in OutBuf to hold a potentially expanded > character > + break; > + } > + return -1; // Something corrupted and filp->devdata is now NULL > } > } > // At this point, the characters to write are in OutBuf > // First, linearize the buffer > - NumWritten = OutBuf->Copy(OutBuf, gMD->UString, UNICODE_STRING_MAX-1); > - gMD->UString[NumWritten] = 0; // Ensure that the buffer is terminated > + NumProc = OutBuf->Copy(OutBuf, gMD->UString, UNICODE_STRING_MAX-1); > + gMD->UString[NumProc] = 0; // Ensure that the buffer is terminated > > 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); > + NumProc = filp->f_ops->fo_write(filp, NULL, NumProc, gMD->UString); > + > + // Consume the output characters > + W_INSTRUMENT OutBuf->Flush(OutBuf, NumProc); > } > else { > // Output device expects narrow characters, convert to MBCS > MbcsPtr = (char *)gMD->UString2; > - // Determine the needed space > + // Determine the needed space. NumProc is the number of bytes needed. > NumProc = (ssize_t)EstimateWtoM((const wchar_t *)gMD->UString, > UNICODE_STRING_MAX * > sizeof(wchar_t), &CharLen); > > - // Now translate this into MBCS in Buffer > - NumWritten = wcstombs(MbcsPtr, (const wchar_t *)gMD->UString, NumProc); > - MbcsPtr[NumWritten] = 0; // Ensure the buffer is terminated > + // Now translate this into MBCS in the buffer pointed to by MbcsPtr. > + // The returned value, NumProc, is the resulting number of bytes. > + NumProc = wcstombs(MbcsPtr, (const wchar_t *)gMD->UString, NumProc); > + MbcsPtr[NumProc] = 0; // Ensure the buffer is terminated > > // Send the MBCS buffer to Output > - NumWritten = filp->f_ops->fo_write(filp, NULL, NumWritten, MbcsPtr); > + NumProc = filp->f_ops->fo_write(filp, NULL, NumProc, MbcsPtr); > + // Mark the Mbcs buffer after the last byte actually written > + MbcsPtr[NumProc] = 0; > + // Count the CHARACTERS actually sent > + CharLen = CountMbcsChars(MbcsPtr); > + > + // Consume the number of output characters actually sent > + W_INSTRUMENT OutBuf->Flush(OutBuf, CharLen); > } > - // Consume the translated characters > - (void)OutBuf->Flush(OutBuf, NumWritten); > } > else { > if(This == NULL) { > @@ -200,7 +244,7 @@ > } > // Otherwise, errno is already set. > } > - return NumWritten; > + return NumConsumed; > } > > /** Echo a character to an output device. > -- > Daryl McDaniel > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
