From ad910059a7501d0f9f7b5f722258cb902c64eb15 Mon Sep 17 00:00:00 2001 From: patacongo Date: Thu, 31 Jul 2008 00:28:24 +0000 Subject: Fix bug: Using unsigned to detect errno<0 git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@791 42af7a65-404d-4744-a932-0658087f49c3 --- nuttx/ChangeLog | 4 ++ nuttx/Documentation/NuttX.html | 6 +- nuttx/examples/pipe/redirect_test.c | 130 +++++++++++++++++++++++++----------- nuttx/fs/fs_close.c | 6 +- nuttx/fs/fs_files.c | 127 +++++++++++++++++++++++++++-------- nuttx/fs/fs_open.c | 20 +++--- nuttx/lib/lib_libfflush.c | 2 +- nuttx/sched/sched_releasefiles.c | 26 ++++---- 8 files changed, 228 insertions(+), 93 deletions(-) (limited to 'nuttx') diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index da542dfe3..39d4fa603 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -381,4 +381,8 @@ * Removed limitation: task_create() was only dup'ing 3 file descriptors (now dups all open file descriptors). * Added a test for redirection of stdio through pipes + * Fixed error in dup and dup2: Must call open/close methods in fs/driver so that + driver can correctly maintain open reference counts. + * Fixed in error in stdio flush logic. Needed ssize_t vs size_t for error + check. diff --git a/nuttx/Documentation/NuttX.html b/nuttx/Documentation/NuttX.html index 19c70498e..e7a60182c 100644 --- a/nuttx/Documentation/NuttX.html +++ b/nuttx/Documentation/NuttX.html @@ -8,7 +8,7 @@

NuttX RTOS

-

Last Updated: July 29, 2008

+

Last Updated: July 30, 2008

@@ -1030,6 +1030,10 @@ nuttx-0.3.12 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr> * Removed limitation: task_create() was only dup'ing 3 file descriptors (now dups all open file descriptors). * Added a test for redirection of stdio through pipes + * Fixed error in dup and dup2: Must call open/close methods in fs/driver so that + driver can correctly maintain open reference counts. + * Fixed in error in stdio flush logic. Needed ssize_t vs size_t for error + check. pascal-0.1.3 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr> diff --git a/nuttx/examples/pipe/redirect_test.c b/nuttx/examples/pipe/redirect_test.c index 6a1f60fab..727dfa4db 100644 --- a/nuttx/examples/pipe/redirect_test.c +++ b/nuttx/examples/pipe/redirect_test.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include "pipe.h" @@ -62,6 +63,8 @@ * Private Data ****************************************************************************/ +static sem_t g_rddone; + /**************************************************************************** * Private Functions ****************************************************************************/ @@ -73,37 +76,49 @@ static int redirect_reader(int argc, char *argv[]) { char buffer[READ_SIZE]; - int fd = (int)argv[0]; + int fdin; + int fdout; int ret; int nbytes = 0; - printf("redirect_reader: started with fd=%s\n", argv[1]); + printf("redirect_reader: started with fdin=%s\n", argv[1]); + + /* Convert the fdin to binary */ - /* Convert the fd to binary */ + fdin = atoi(argv[1]); + fdout = atoi(argv[2]); - fd = atoi(argv[1]); + /* Close fdout -- we don't need it */ + + ret = close(fdout); + if (ret != 0) + { + fprintf(stderr, "redirect_reader: failed to close fdout=%d\n", fdout); + return 1; + } - /* Re-direct the fd to stdin */ + /* Re-direct the fdin to stdin */ - ret = dup2(fd, 0); + ret = dup2(fdin, 0); if (ret != 0) { fprintf(stderr, "redirect_reader: dup2 failed: %d\n", errno); - close(fd); - return 1; + close(fdin); + return 2; } /* Close the original file descriptor */ - ret = close(fd); + ret = close(fdin); if (ret != 0) { - fprintf(stderr, "redirect_reader: failed to close fd=%d\n", fd); - return 2; + fprintf(stderr, "redirect_reader: failed to close fdin=%d\n", fdin); + return 3; } /* Then read from stdin until we hit the end of file */ + fflush(stdout); for (;;) { /* Read from stdin */ @@ -112,7 +127,7 @@ static int redirect_reader(int argc, char *argv[]) if (ret < 0 ) { fprintf(stderr, "redirect_reader: read failed, errno=%d\n", errno); - return 3; + return 4; } else if (ret == 0) { @@ -126,7 +141,7 @@ static int redirect_reader(int argc, char *argv[]) if (ret < 0) { fprintf(stderr, "redirect_reader: read failed, errno=%d\n", errno); - return 4; + return 5; } } @@ -134,9 +149,11 @@ static int redirect_reader(int argc, char *argv[]) ret = close(0); if (ret != 0) { - fprintf(stderr, "redirect_reader: failed to close fd=%d\n", fd); - return 5; + fprintf(stderr, "redirect_reader: failed to close fd=0\n"); + return 6; } + + sem_post(&g_rddone); printf("redirect_reader: Returning success\n"); return 0; } @@ -147,36 +164,48 @@ static int redirect_reader(int argc, char *argv[]) static int redirect_writer(int argc, char *argv[]) { - int fd; + int fdin; + int fdout; int nbytes = 0; int ret; - printf("redirect_writer: started with fd=%s\n", argv[1]); + fprintf(stderr, "redirect_writer: started with fdout=%s\n", argv[2]); + + /* Convert the fdout to binary */ + + fdin = atoi(argv[1]); + fdout = atoi(argv[2]); - /* Convert the fd to binary */ + /* Close fdin -- we don't need it */ - fd = atoi(argv[1]); + ret = close(fdin); + if (ret != 0) + { + fprintf(stderr, "redirect_reader: failed to close fdin=%d\n", fdin); + return 1; + } - /* Re-direct the fd to stdout */ + /* Re-direct the fdout to stdout */ - ret = dup2(fd, 1); + ret = dup2(fdout, 1); if (ret != 0) { fprintf(stderr, "redirect_writer: dup2 failed: %d\n", errno); - return 1; + return 2; } /* Close the original file descriptor */ - ret = close(fd); + ret = close(fdout); if (ret != 0) { - fprintf(stderr, "redirect_reader: failed to close fd=%d\n", fd); - return 2; + fprintf(stderr, "redirect_reader: failed to close fdout=%d\n", fdout); + return 3; } /* Then write a bunch of stuff to stdout */ + fflush(stderr); nbytes += printf("\nFour score and seven years ago our fathers brought forth on this continent a new nation,\n"); nbytes += printf("conceived in Liberty, and dedicated to the proposition that all men are created equal.\n"); nbytes += printf("\nNow we are engaged in a great civil war, testing whether that nation, or any nation, so\n"); @@ -194,15 +223,18 @@ static int redirect_writer(int argc, char *argv[]) nbytes += printf("here highly resolve that these dead shall not have died in vain—that this nation, under God,\n"); nbytes += printf("shall have a new birth of freedom—and that government of the people, by the people, for the\n"); nbytes += printf("people, shall not perish from the earth.\n\n"); - printf("redirect_writer: %d bytes read\n", nbytes); + fflush(stdout); + + fprintf(stderr, "redirect_writer: %d bytes written\n", nbytes); ret = close(1); if (ret != 0) { - fprintf(stderr, "redirect_writer: failed to close fd=%d\n", fd); - return 3; + fprintf(stderr, "redirect_writer: failed to close fd=1\n"); + return 4; } - printf("redirect_writer: Returning success\n"); + + fprintf(stderr, "redirect_writer: Returning success\n"); return 0; } @@ -216,14 +248,17 @@ static int redirect_writer(int argc, char *argv[]) int redirection_test(void) { - const char *argv[2]; - char buffer[8]; + const char *argv[3]; + char buffer1[8]; + char buffer2[8]; int readerid; int writerid; int filedes[2]; int ret; - /* Create the pipe */ + sem_init(&g_rddone, 0, 0); + + /* Create the pipe */ ret = pipe(filedes); if (ret < 0) @@ -232,12 +267,15 @@ int redirection_test(void) return 5; } + sprintf(buffer1, "%d", filedes[0]); + argv[0] = buffer1; + sprintf(buffer2, "%d", filedes[1]); + argv[1] = buffer2; + argv[2] = NULL; + /* Start redirect_reader thread */ printf("redirection_test: Starting redirect_reader task with fd=%d\n", filedes[0]); - sprintf(buffer, "%d", filedes[0]); - argv[0] = buffer; - argv[1] = NULL; readerid = task_create("redirect_reader", 50, CONFIG_EXAMPLES_PIPE_STACKSIZE, redirect_reader, argv); if (readerid < 0) { @@ -248,9 +286,6 @@ int redirection_test(void) /* Start redirect_writer task */ printf("redirection_test: Starting redirect_writer task with fd=%d\n", filedes[1]); - sprintf(buffer, "%d", filedes[1]); - argv[0] = buffer; - argv[1] = NULL; writerid = task_create("redirect_writer", 50, CONFIG_EXAMPLES_PIPE_STACKSIZE, redirect_writer, argv); if (writerid < 0) { @@ -263,10 +298,27 @@ int redirection_test(void) return 2; } + /* We should be able to close the pipe file descriptors now. */ + + if (close(filedes[0]) != 0) + { + fprintf(stderr, "user_start: close failed: %d\n", errno); + } + if (close(filedes[1]) != 0) + { + fprintf(stderr, "user_start: close failed: %d\n", errno); + } + + if (ret != 0) + { + fprintf(stderr, "user_start: PIPE test FAILED (%d)\n", ret); + return 6; + } + /* Wait for redirect_writer thread to complete */ printf("redirection_test: Waiting...\n"); - sleep(10); + sem_wait(&g_rddone); printf("redirection_test: returning %d\n", ret); return ret; diff --git a/nuttx/fs/fs_close.c b/nuttx/fs/fs_close.c index c1f3322cc..736788a2e 100644 --- a/nuttx/fs/fs_close.c +++ b/nuttx/fs/fs_close.c @@ -1,7 +1,7 @@ /**************************************************************************** - * fs_close.c + * fs/fs_close.c * - * Copyright (C) 2007 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -14,7 +14,7 @@ * notice, this list of conditions and the following disclaimer in * the documentation and/or other materials provided with the * distribution. - * 3. Neither the name Gregory Nutt nor the names of its contributors may be + * 3. Neither the name NuttX nor the names of its contributors may be * used to endorse or promote products derived from this software * without specific prior written permission. * diff --git a/nuttx/fs/fs_files.c b/nuttx/fs/fs_files.c index 8ed7c615e..6f4941dce 100644 --- a/nuttx/fs/fs_files.c +++ b/nuttx/fs/fs_files.c @@ -1,7 +1,7 @@ -/************************************************************ - * fs_files.c +/**************************************************************************** + * fs/s_files.c * - * Copyright (C) 2007 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -31,11 +31,11 @@ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE * POSSIBILITY OF SUCH DAMAGE. * - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Included Files - ************************************************************/ + ****************************************************************************/ #include #include @@ -47,25 +47,25 @@ #include #include "fs_internal.h" -/************************************************************ +/**************************************************************************** * Definitions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Types - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ static void _files_semtake(FAR struct filelist *list) { @@ -83,9 +83,9 @@ static void _files_semtake(FAR struct filelist *list) #define _files_semgive(list) sem_post(&list->fl_sem) -/************************************************************ +/**************************************************************************** * Pulblic Functions - ************************************************************/ + ****************************************************************************/ /* This is called from the FS initialization logic to configure * the files. @@ -191,42 +191,117 @@ int files_releaselist(FAR struct filelist *list) int files_dup(FAR struct file *filep1, FAR struct file *filep2) { FAR struct filelist *list; + FAR struct inode *inode; + int err; + int ret; if (!filep1 || !filep1->f_inode || !filep2) { - *get_errno_ptr() = EBADF; - return ERROR; + err = EBADF; + goto errout; } +#ifndef CONFIG_DISABLE_MOUNTPOINT + if (INODE_IS_MOUNTPT(filep1->f_inode)) + { + err = ENOSYS; /* Not yet supported */ + goto errout; + } +#endif + list = sched_getfiles(); if (!list) { - *get_errno_ptr() = EMFILE; - return ERROR; + err = EMFILE; + goto errout; } _files_semtake(list); /* If there is already an inode contained in the new file structure, - * release it (effectively closing the file). + * close the file and release the inode. */ - if (filep2->f_inode) + inode = filep2->f_inode; + if (inode) { + /* Close the file, driver, or mountpoint. */ + + if (inode->u.i_ops && inode->u.i_ops->close) + { + /* Perform the close operation */ + + ret = inode->u.i_ops->close(filep2); + if (ret < 0) + { + /* An error occurred while closing the driver */ + + goto errout_with_ret; + } + } + + /* Release the inode */ + inode_release(filep2->f_inode); } /* Increment the reference count on the contained inode */ - inode_addref(filep1->f_inode); + inode = filep1->f_inode; + inode_addref(inode); /* Then clone the file structure */ filep2->f_oflags = filep1->f_oflags; filep2->f_pos = filep1->f_pos; - filep2->f_inode = filep1->f_inode; + filep2->f_inode = inode; + + /* Call the open method on the file, driver, mountpoint so that it + * can maintain the correct open counts. + */ + + if (inode->u.i_ops && inode->u.i_ops->open) + { +#ifndef CONFIG_DISABLE_MOUNTPOINT +#if 0 /* Not implemented */ + if (INODE_IS_MOUNTPT(inode)) + { + /* Open a file on the mountpoint */ + + ret = inode->u.i_mops->open(filep2, ?, filep2->f_oflags, ?); + } + else +#endif +#endif + { + /* Open the psuedo file or device driver */ + + ret = inode->u.i_ops->open(filep2); + } + + /* Handle open failures */ + + if (ret < 0) + { + goto errout_with_inode; + } + } _files_semgive(list); return OK; + +/* Handler various error conditions */ + +errout_with_inode: + inode_release(filep2->f_inode); + filep2->f_oflags = 0; + filep2->f_pos = 0; + filep2->f_inode = NULL; +errout_with_ret: + err = -ret; + _files_semgive(list); +errout: + errno = err; + return ERROR; } /* Allocate a struct files instance and associate it with an diff --git a/nuttx/fs/fs_open.c b/nuttx/fs/fs_open.c index d07b73416..166471bfb 100644 --- a/nuttx/fs/fs_open.c +++ b/nuttx/fs/fs_open.c @@ -1,7 +1,7 @@ -/************************************************************ +/**************************************************************************** * fs_open.c * - * Copyright (C) 2007 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -14,7 +14,7 @@ * notice, this list of conditions and the following disclaimer in * the documentation and/or other materials provided with the * distribution. - * 3. Neither the name Gregory Nutt nor the names of its contributors may be + * 3. Neither the name NuttX nor the names of its contributors may be * used to endorse or promote products derived from this software * without specific prior written permission. * @@ -31,11 +31,11 @@ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE * POSSIBILITY OF SUCH DAMAGE. * - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Included Files - ************************************************************/ + ****************************************************************************/ #include #include @@ -48,13 +48,13 @@ #include #include "fs_internal.h" -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Functions - ************************************************************/ + ****************************************************************************/ int inode_checkflags(FAR struct inode *inode, int oflags) { diff --git a/nuttx/lib/lib_libfflush.c b/nuttx/lib/lib_libfflush.c index 52af91c8d..23be9c83a 100644 --- a/nuttx/lib/lib_libfflush.c +++ b/nuttx/lib/lib_libfflush.c @@ -106,7 +106,7 @@ ssize_t lib_fflush(FILE *stream, boolean bforce) { #if CONFIG_STDIO_BUFFER_SIZE > 0 const unsigned char *src; - size_t bytes_written; + ssize_t bytes_written; size_t nbuffer; /* Return EBADF if the file is not opened for writing */ diff --git a/nuttx/sched/sched_releasefiles.c b/nuttx/sched/sched_releasefiles.c index 9edf8ecd9..160b0fb42 100644 --- a/nuttx/sched/sched_releasefiles.c +++ b/nuttx/sched/sched_releasefiles.c @@ -1,7 +1,7 @@ -/************************************************************ - * sched_releasefiles.c +/**************************************************************************** + * sched/sched_releasefiles.c * - * Copyright (C) 2007 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -14,7 +14,7 @@ * notice, this list of conditions and the following disclaimer in * the documentation and/or other materials provided with the * distribution. - * 3. Neither the name Gregory Nutt nor the names of its contributors may be + * 3. Neither the name NuttX nor the names of its contributors may be * used to endorse or promote products derived from this software * without specific prior written permission. * @@ -31,11 +31,11 @@ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE * POSSIBILITY OF SUCH DAMAGE. * - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Included Files - ************************************************************/ + ****************************************************************************/ #include @@ -46,15 +46,15 @@ #if CONFIG_NFILE_DESCRIPTORS > 0 || CONFIG_NSOCKET_DESCRIPTORS > 0 -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Function: sched_releasefiles * * Description: @@ -68,7 +68,7 @@ * * Assumptions: * - ************************************************************/ + ****************************************************************************/ int sched_releasefiles(_TCB *tcb) { -- cgit v1.2.3