Please also note in the comments for the commits where you fixed indentation 
issues.

Reviewed-by: Erik Bjorge <[email protected]>

> -----Original Message-----
> From: Daryl McDaniel [mailto:[email protected]]
> Sent: Sunday, January 3, 2016 2:51 PM
> To: [email protected]; Carsey, Jaben <[email protected]>; Bjorge, 
> Erik C
> <[email protected]>
> Subject: [PATCH] StdLib: Clarify and improve comments.
> 
> StdLib: Clarify and improve comments.
> 
> LibC/Locale/multibyte_Utf8.c
> LibC/Uefi/SysCalls.c
>   Clarify and improve comments.
> 
> Include/sys/termios.h
>   Add parameter names to function prototypes as referenced in the comments.
> 
> StdLibPrivateInternalFiles\Include\kfile.h
>   Add comment for the fo_close fileop.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Daryl McDaniel <[email protected]>
> 
> ---
>   .../StdLib/LibC/Locale/multibyte_Utf8.c         |  20 +++++++++++---------
>   .../StdLib/LibC/Uefi/SysCalls.c                 | 127 +++++++------
>   .../StdLib/Include/sys/termios.h                |  11 ++++++-----
>   .../StdLibPrivateInternalFiles\Include\kfile.h  |  11 +++++++++--
>   4 files changed, 95 insertions(+), 74 deletions(-)
> 
> diff U3 a/StdLib/LibC/Locale/multibyte_Utf8.c 
> b/StdLib/LibC/Locale/multibyte_Utf8.c
> --- a/StdLib/LibC/Locale/multibyte_Utf8.c Tue May 14 17:59:11 2013
> +++ b/StdLib/LibC/Locale/multibyte_Utf8.c Sun Jan 03 12:45:02 2016
> @@ -1,4 +1,5 @@
>  /** @file
> +  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
> @@ -16,7 +17,7 @@
>  #include  <sys/types.h>
>  #include  <limits.h>
> 
> -typedef      int      ch_UCS4;
> +typedef int      ch_UCS4;
> 
>  static  mbstate_t     LocalConvState = {0};
> 
> @@ -79,7 +80,7 @@
>      // We are in an invalid state
>      ps->A = 0;    // Initial State
>    }
> -  ps->C[ps->A] = ch;  // Save the current character
> +  ps->C[ps->A] = ch;  // Save the current byte
>    Mask = utf8_code_length[ch];
> 
>    if(ps->A == 0) {    // Initial State.  First byte of sequence.
> @@ -89,7 +90,7 @@
>        case 0:                       // State 0, Code 0
>          errno = EILSEQ;
>          RetVal = -1;
> -        ps->E = 1;        // Consume this character
> +        ps->E = 1;        // Consume this byte
>          break;
>        case 1:                       // State 0, Code 1
>          // ASCII-7 Character
> @@ -116,7 +117,7 @@
>              ps->A = 0;      // Next state is State-0
>              RetVal = 2;
>            }
> -          else {    // This isn't the last character, get more.  State 1, 
> Code 3 or 4
> +          else {    // This isn't the last byte, get more.  State 1, Code 3 
> or 4
>              ps->A = 2;
>              RetVal = -2;
>            }
> @@ -158,12 +159,12 @@
>              errno = EILSEQ;
>              ps->A = 0;
>              RetVal = -1;
> -            ps->E = 4;      // Can't happen, but consume this character 
> anyway
> +            ps->E = 4;      // Can't happen, but consume this byte anyway
>            }
>            break;
>        }
>      }
> -    else {                // Invalid surrogate character
> +    else {                // Invalid surrogate byte
>        errno = EILSEQ;
>        ps->A = 0;          // Next is State-0
>        RetVal = -1;
> @@ -287,7 +288,8 @@
> 
>      @param[in]    Src       Pointer to a wide character string.
>      @param[in]    Limit     Maximum number of bytes the converted string may 
> occupy.
> -    @param[out]   NumChar   Pointer to where to store the number of wide 
> characters, or
> NULL.
> +    @param[out]   NumChar   Pointer to where to store the number of wide 
> characters
> +                            consumed, or NULL.
> 
>      @return     The number of bytes required to convert Src to MBCS,
>                  not including the terminating NUL.  If NumChar is not NULL, 
> the number
> @@ -961,8 +963,8 @@
>      @return   If a wide character is encountered that does not correspond to 
> a
>                valid multibyte character, the wcstombs function returns
>                (size_t)(-1). Otherwise, the wcstombs function returns the 
> number
> -              of bytes modified, not including a terminating null character,
> -              if any.
> +              of bytes in the resulting multibyte character sequence,
> +              not including the terminating null character (if any).
> 
>      Declared in: stdlib.h
>  **/
> diff U3 a/StdLib/LibC/Uefi/SysCalls.c b/StdLib/LibC/Uefi/SysCalls.c
> --- a/StdLib/LibC/Uefi/SysCalls.c Fri Dec 21 10:19:41 2012
> +++ b/StdLib/LibC/Uefi/SysCalls.c Sun Jan 03 12:52:28 2016
> @@ -1,6 +1,7 @@
>  /** @file
>    EFI versions of NetBSD system calls.
> 
> +  Copyright (c) 2016, Daryl McDaniel. All rights reserved.<BR>
>    Copyright (c) 2010 - 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 that accompanies this 
> distribution.
> @@ -192,18 +193,18 @@
>      Fp = &gMD->fdarray[fd];
>      // Check if there are other users of this FileHandle
>      if(Fp->RefCount == 1) { // There should be no other users
> -    if(! IsDupFd(fd)) {
> -      // Only do the close if no one else is using the FileHandle
> -      if(Fp->f_iflags & FIF_DELCLOSE) {
> -        /* Handle files marked "Delete on Close". */
> -        if(Fp->f_ops->fo_delete != NULL) {
> -          retval = Fp->f_ops->fo_delete(Fp);
> +      if(! IsDupFd(fd)) {
> +        // Only do the close if no one else is using the FileHandle
> +        if(Fp->f_iflags & FIF_DELCLOSE) {
> +          /* Handle files marked "Delete on Close". */
> +          if(Fp->f_ops->fo_delete != NULL) {
> +            retval = Fp->f_ops->fo_delete(Fp);
> +          }
>          }
> -      }
> -      else {
> +        else {
>            retval = Fp->f_ops->fo_close( Fp);
> +        }
>        }
> -    }
>        Fp->f_iflags = NewState;    // Close this FD or reserve it
>        Fp->RefCount = 0;           // No one using this FD
>      }
> @@ -671,58 +672,58 @@
> 
>    Status = ParsePath(path, &NewPath, &Node, &Instance, &MPath);
>    if(Status == RETURN_SUCCESS) {
> -    if((Node == NULL)               ||
> -       (Node->InstanceList == NULL))
> +    if ((Node == NULL)               ||
> +        (Node->InstanceList == NULL))
>      {
>        errno   = EPERM;
>      }
>      else {
> -  // Could add a test to see if the file name begins with a period.
> -  // If it does, then add the HIDDEN flag to Attributes.
> +      // Could add a test to see if the file name begins with a period.
> +      // If it does, then add the HIDDEN flag to Attributes.
> 
> -  // Get an available fd
> +      // Get an available fd
>        fd = FindFreeFD( VALID_CLOSED );
> 
> -  if( fd < 0 ) {
> -    // All available FDs are in use
> -    errno = EMFILE;
> -  }
> +      if( fd < 0 ) {
> +        // All available FDs are in use
> +        errno = EMFILE;
> +      }
>        else {
> -      filp = &gMD->fdarray[fd];
> -      // Save the flags and mode in the File Descriptor
> -      filp->Oflags = oflags;
> -      filp->Omode = mode;
> +        filp = &gMD->fdarray[fd];
> +        // Save the flags and mode in the File Descriptor
> +        filp->Oflags = oflags;
> +        filp->Omode = mode;
> 
>          doresult = Node->OpenFunc(Node, filp, Instance, NewPath, MPath);
> -      if(doresult < 0) {
> -        filp->f_iflags = 0;   // Release this FD
> -        fd = -1;              // Indicate an error
> -      }
> -      else {
> -        // Build our final f_iflags value
> -        OpenMode  = ( mode & S_ACC_READ )  ? S_ACC_READ : 0;
> -        OpenMode |= ( mode & S_ACC_WRITE ) ? S_ACC_WRITE : 0;
> -
> -        filp->f_iflags |= OpenMode;
> -
> -        if((oflags & O_TTY_INIT) && (filp->f_iflags & _S_ITTY) && 
> (filp->devdata !=
> NULL)) {
> -          // Initialize the device's termios flags to a "sane" value
> -          Termio = &((cIIO *)filp->devdata)->Termio;
> -          Termio->c_iflag = ICRNL | IGNSPEC;
> -          Termio->c_oflag = OPOST | ONLCR | OXTABS | ONOEOT | ONOCR | ONLRET 
> | OCTRL;
> -          Termio->c_lflag = ECHO | ECHOE | ECHONL | ICANON;
> -          Termio->c_cc[VERASE]  = 0x08;   // ^H Backspace
> -          Termio->c_cc[VKILL]   = 0x15;   // ^U
> -          Termio->c_cc[VINTR]   = 0x03;   // ^C Interrupt character
> +        if(doresult < 0) {
> +          filp->f_iflags = 0;   // Release this FD
> +          fd = -1;              // Indicate an error
>          }
> -        ++filp->RefCount;
> -        FILE_SET_MATURE(filp);
> -      }
> +        else {
> +          // Build our final f_iflags value
> +          OpenMode  = ( mode & S_ACC_READ )  ? S_ACC_READ : 0;
> +          OpenMode |= ( mode & S_ACC_WRITE ) ? S_ACC_WRITE : 0;
> +
> +          filp->f_iflags |= OpenMode;
> +
> +          if((oflags & O_TTY_INIT) && (filp->f_iflags & _S_ITTY) && 
> (filp->devdata !=
> NULL)) {
> +            // Initialize the device's termios flags to a "sane" value
> +            Termio = &((cIIO *)filp->devdata)->Termio;
> +            Termio->c_iflag = ICRNL | IGNSPEC;
> +            Termio->c_oflag = OPOST | ONLCR | OXTABS | ONOEOT | ONOCR | 
> ONLRET | OCTRL;
> +            Termio->c_lflag = ECHO | ECHOE | ECHONL | ICANON;
> +            Termio->c_cc[VERASE]  = 0x08;   // ^H Backspace
> +            Termio->c_cc[VKILL]   = 0x15;   // ^U
> +            Termio->c_cc[VINTR]   = 0x03;   // ^C Interrupt character
>            }
> +          ++filp->RefCount;
> +          FILE_SET_MATURE(filp);
> +        }
> +      }
>      }
>      free(NewPath);
> -        }
> -    free(MPath);    // We don't need this any more.
> +  }
> +  free(MPath);    // We don't need this any more.
> 
>    // return the fd of our now open file
>    return fd;
> @@ -1211,21 +1212,30 @@
> 
>      This function writes the specified number of bytes to the file at the 
> current
>      file position. The current file position is advanced the actual number 
> of bytes
> -    written, which is returned in BufferSize. Partial writes only occur when 
> there
> -    has been a data error during the write attempt (such as "volume space 
> full").
> -    The file is automatically grown to hold the data if required. Direct 
> writes to
> -    opened directories are not supported.
> +    written. Partial writes only occur when there has been a data error 
> during
> +    the write attempt (such as "volume space full").  The file is 
> automatically
> +    grown to hold the data if required.
> +
> +    Direct writes to opened directories are not supported.
> 
>      If fildes refers to a terminal device, isatty() returns TRUE, a partial 
> write
>      will occur if a NULL or EOF character is encountered before n characters 
> have
> -    been written.  Characters inserted due to line-end translations will not 
> be
> -    counted.  Unconvertable characters are translated into the UEFI character
> -    BLOCKELEMENT_LIGHT_SHADE.
> +    been written.  Characters inserted due to line-end translations or TAB
> +    expansion will not be counted.  Unconvertable characters are translated 
> into
> +    the UEFI character BLOCKELEMENT_LIGHT_SHADE.
> 
>      Since the UEFI console device works on wide characters, the buffer is 
> assumed
> -    to contain a single-byte character stream which is then translated to 
> wide
> -    characters using the mbtowc() functions.  The resulting wide character 
> stream
> -    is what is actually sent to the UEFI console.
> +    to contain a byte-oriented multi-byte character stream which is then
> +    translated to wide characters using the mbtowc() functions.  The 
> resulting
> +    wide character stream is what is actually sent to the UEFI console.
> +
> +    Although both text and binary wide-oriented streams are conceptually
> +    sequences of wide characters, the external file associated with a
> +    wide-oriented stream is a sequence of multibyte characters,
> +    generalized as follows:
> +      - Multibyte encodings within files may contain embedded null bytes
> +        (unlike multibyte encodings valid for use internal to the program).
> +      - A file need not begin nor end in the initial shift state.
> 
>      @param[in]  fd      Descriptor of file to be written to.
>      @param[in]  buf     Pointer to data to write to the file.
> @@ -1250,10 +1260,11 @@
>        IIO = filp->devdata;
>        if(isatty(fd) && (IIO != NULL)) {
>          // Output to an Interactive I/O device
> +        // (Terminal device or the slave side of a pseudo-tty)
>          BufSize = IIO->Write(filp, buf, nbyte);
>        }
>        else {
> -        // Output to a file, socket, pipe, etc.
> +        // Output to a regular file, socket, pipe, etc.
>          BufSize = filp->f_ops->fo_write(filp, &filp->f_offset, nbyte, buf);
>        }
>      }
> diff U3 a/StdLib/Include/sys/termios.h b/StdLib/Include/sys/termios.h
> --- a/StdLib/Include/sys/termios.h  Tue Oct 28 11:20:48 2014
> +++ b/StdLib/Include/sys/termios.h  Sun Jan 03 12:45:02 2016
> @@ -2,6 +2,7 @@
>      Macros and declarations for terminal oriented ioctls and
>      I/O discipline.
> 
> +    Copyright (c) 2016, Daryl McDaniel. All rights reserved.<BR>
>      Copyright (c) 2012 - 2014, 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 that accompanies this 
> distribution.
> @@ -288,7 +289,7 @@
>                      * EBADF - The fd argument is not a valid file descriptor.
>                      * ENOTTY - The file associated with fd is not an 
> interactive IO
> device.
>  **/
> -int     tcgetattr   (int, struct termios *);
> +int     tcgetattr   (int fd, struct termios *pTermios);
> 
>  /** Set the parameters associated with an interactive IO device.
> 
> @@ -314,7 +315,7 @@
>                      * EBADF - The fd argument is not a valid file descriptor.
>                      * ENOTTY - The file associated with fd is not an 
> interactive IO
> device.
>  **/
> -int     tcsetattr   (int, int, const struct termios *);
> +int     tcsetattr   (int fd, int OptAct, const struct termios *pTermios);
> 
>  /** Transmit pending output.
> 
> @@ -328,7 +329,7 @@
>                      * EINTR - A signal interrupted tcdrain().
>                      * ENOTSUP - This function is not supported.
>  **/
> -int     tcdrain     (int);
> +int     tcdrain     (int fd);
> 
>  /** Suspend or restart the transmission or reception of data.
> 
> @@ -353,7 +354,7 @@
>                      * EINVAL - The Action argument is not a supported value.
>                      * ENOTSUP - This function is not supported.
>  **/
> -int     tcflow      (int, int);
> +int     tcflow      (int fd, int Action);
> 
>  /** Discard non-transmitted output data, non-read input data, or both.
> 
> @@ -375,7 +376,7 @@
>                      * EINVAL - The QueueSelector argument is not a supported 
> value.
>                      * ENOTSUP - This function is not supported.
>  **/
> -int     tcflush     (int, int);
> +int     tcflush     (int fd, int QueueSelector);
> 
>  //int     tcsendbreak (int, int);
>  //pid_t   tcgetsid    (int);
> diff U3 a/StdLibPrivateInternalFiles/Include/kfile.h
> b/StdLibPrivateInternalFiles/Include/kfile.h
> --- a/StdLibPrivateInternalFiles/Include/kfile.h  Thu Sep 20 13:01:21 2012
> +++ b/StdLibPrivateInternalFiles/Include/kfile.h  Sun Jan 03 12:47:17 2016
> @@ -1,6 +1,7 @@
>  /** @file
>    The EFI kernel's interpretation of a "file".
> 
> +  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
> @@ -101,8 +102,14 @@
>  };
> 
>  struct fileops {
> -  /* These functions must always be implemented. */
> +/* These functions must always be implemented. */
> +
> +  /** Perform device specific operations for closing the device.
> +      It is the responsibility of this function to flush or discard
> +      buffer contents.
> +  **/
>    int     (EFIAPI *fo_close)    (struct __filedes *filp);
> +
>    ssize_t (EFIAPI *fo_read)     (struct __filedes *filp, off_t *Offset, 
> size_t Len, void
> *Buf);
>    ssize_t (EFIAPI *fo_write)    (struct __filedes *filp, off_t *Offset, 
> size_t Len, const
> void *Buf);
> 
> @@ -119,7 +126,7 @@
>    int     (EFIAPI *fo_mkdir)    (const char *path, __mode_t perms);
>    int     (EFIAPI *fo_rename)   (const char *from, const char *to);
> 
> -  /* Use a NULL if this function has not been implemented by the device. */
> +/* Use a NULL if this function has not been implemented by the device. */
>    off_t   (EFIAPI *fo_lseek)    (struct __filedes *filp, off_t, int);
>  };
> --
> Daryl McDaniel
> 
> 

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to