Guys, 

Thanks a lot! New patch works fine for us.

Best regards,
Alexey Vegner
http://www.paragon-software.com

-----Original Message-----
From: Bjorge, Erik C [mailto:[email protected]] 
Sent: Monday, January 4, 2016 8:36 PM
To: Daryl McDaniel <[email protected]>; Alexey Vegner 
<[email protected]>; [email protected] 
<[email protected]>; Carsey, Jaben <[email protected]>
Subject: RE: [PATCH] StdLib: Fix IIO_Write() to return the number of bytes 
consumed, not characters output.

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

Reply via email to