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

