From a39ce80add0cd62ba4850b3e41ebbabc44eeff49 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Tue, 5 Jul 2016 12:12:44 -0600 Subject: [PATCH] SYSLOG logic should use existing file interfaces, not re-invent them. --- configs/nucleo-f4x1re/src/stm32_ajoystick.c | 3 +- configs/nucleo-l476rg/src/stm32_ajoystick.c | 3 +- configs/sama5d3-xplained/src/sam_ajoystick.c | 3 +- drivers/syslog/syslog_device.c | 52 ++++------------ fs/vfs/fs_ioctl.c | 65 ++++++++++++++------ include/nuttx/fs/fs.h | 20 ++++++ 6 files changed, 85 insertions(+), 61 deletions(-) diff --git a/configs/nucleo-f4x1re/src/stm32_ajoystick.c b/configs/nucleo-f4x1re/src/stm32_ajoystick.c index 432d91667b..a40f08f4b0 100644 --- a/configs/nucleo-f4x1re/src/stm32_ajoystick.c +++ b/configs/nucleo-f4x1re/src/stm32_ajoystick.c @@ -206,7 +206,8 @@ static int ajoy_sample(FAR const struct ajoy_lowerhalf_s *lower, * channels are enabled). */ - nread = file_read(&g_adcfile, adcmsg, MAX_ADC_CHANNELS * sizeof(struct adc_msg_s)); + nread = file_read(&g_adcfile, adcmsg, + MAX_ADC_CHANNELS * sizeof(struct adc_msg_s)); if (nread < 0) { int errcode = get_errno(); diff --git a/configs/nucleo-l476rg/src/stm32_ajoystick.c b/configs/nucleo-l476rg/src/stm32_ajoystick.c index 6f49fd9823..71a61ef417 100644 --- a/configs/nucleo-l476rg/src/stm32_ajoystick.c +++ b/configs/nucleo-l476rg/src/stm32_ajoystick.c @@ -205,7 +205,8 @@ static int ajoy_sample(FAR const struct ajoy_lowerhalf_s *lower, * channels are enabled). */ - nread = file_read(&g_adcfile, adcmsg, MAX_ADC_CHANNELS * sizeof(struct adc_msg_s)); + nread = file_read(&g_adcfile, adcmsg, + MAX_ADC_CHANNELS * sizeof(struct adc_msg_s)); if (nread < 0) { int errcode = get_errno(); diff --git a/configs/sama5d3-xplained/src/sam_ajoystick.c b/configs/sama5d3-xplained/src/sam_ajoystick.c index 15b53d3f70..7ff2f02195 100644 --- a/configs/sama5d3-xplained/src/sam_ajoystick.c +++ b/configs/sama5d3-xplained/src/sam_ajoystick.c @@ -187,7 +187,8 @@ static int ajoy_sample(FAR const struct ajoy_lowerhalf_s *lower, * channels are enabled). */ - nread = file_read(&g_adcfile, adcmsg, MAX_ADC_CHANNELS * sizeof(struct adc_msg_s)); + nread = file_read(&g_adcfile, adcmsg, + MAX_ADC_CHANNELS * sizeof(struct adc_msg_s)); if (nread < 0) { int errcode = get_errno(); diff --git a/drivers/syslog/syslog_device.c b/drivers/syslog/syslog_device.c index 1350b9084e..c3bf74834a 100644 --- a/drivers/syslog/syslog_device.c +++ b/drivers/syslog/syslog_device.c @@ -133,9 +133,9 @@ static inline int syslog_dev_takesem(void) int ret; /* Does this thread already hold the semaphore? That could happen if - * we wer called recursively, i.e., if the logic kicked off by - * syslog_dev_write() where to generate more debug output. Return an error - * in that case. + * we were called recursively, i.e., if the logic kicked off by + * file_write() where to generate more debug output. Return an + * error in that case. */ if (g_syslog_dev.sl_holder == me) @@ -184,26 +184,6 @@ static inline void syslog_dev_givesem(void) sem_post(&g_syslog_dev.sl_sem); } -/**************************************************************************** - * Name: syslog_dev_write - * - * Description: - * Write to the syslog device - * - ****************************************************************************/ - -static inline ssize_t syslog_dev_write(FAR const void *buf, size_t nbytes) -{ - FAR struct inode *inode; - - /* Let the driver perform the write */ - - inode = g_syslog_dev.sl_file.f_inode; - DEBUGASSERT(inode != NULL && inode->u.i_ops->write != NULL); - - return inode->u.i_ops->write(&g_syslog_dev.sl_file, buf, nbytes); -} - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -505,7 +485,7 @@ int syslog_dev_putc(int ch) if (ret < 0) { /* We probably already hold the semaphore and were probably - * re-entered by the logic kicked off by syslog_dev_write(). + * re-entered by the logic kicked off by file_write(). * We might also have been interrupted by a signal. Either * way, we are outta here. */ @@ -520,7 +500,7 @@ int syslog_dev_putc(int ch) { /* Write the CR-LF sequence */ - nbytes = syslog_dev_write(g_syscrlf, 2); + nbytes = file_write(&g_syslog_dev.sl_file, g_syscrlf, 2); /* Synchronize the file when each CR-LF is encountered (i.e., * implements line buffering always). @@ -538,7 +518,7 @@ int syslog_dev_putc(int ch) /* Write the non-newline character (and don't flush) */ uch = (uint8_t)ch; - nbytes = syslog_dev_write(&uch, 1); + nbytes = file_write(&g_syslog_dev.sl_file, &uch, 1); } syslog_dev_givesem(); @@ -576,24 +556,16 @@ errout_with_errcode: int syslog_dev_flush(void) { - int ret = 0;; - #if defined(CONFIG_SYSLOG_FILE) && !defined(CONFIG_DISABLE_MOUNTPOINT) - FAR struct inode *inode = g_syslog_dev.sl_file.f_inode; + /* Ignore return value, always return success. file_fsync() could fail + * because the file is not open, the inode is not a mountpoint, or the + * mountpoint does not support the sync() method. + */ - /* Is this a mountpoint? Does it support the sync method? */ - - if (inode != NULL && /* File opened (i.e., has inode)? */ - INODE_IS_MOUNTPT(inode) && /* Inode is a mountpoint? */ - inode->u.i_mops->sync != NULL) /* And supports synce method? */ - { - /* Yes... synchronize to the stream */ - - ret = inode->u.i_mops->sync(&g_syslog_dev.sl_file); - } + (void)file_fsync(&g_syslog_dev.sl_file); #endif - return ret; + return OK; } #endif /* CONFIG_NFILE_DESCRIPTORS > 0 */ diff --git a/fs/vfs/fs_ioctl.c b/fs/vfs/fs_ioctl.c index 238da907a7..fda5be76cd 100644 --- a/fs/vfs/fs_ioctl.c +++ b/fs/vfs/fs_ioctl.c @@ -1,7 +1,7 @@ /**************************************************************************** * fs/vfs/fs_ioctl.c * - * Copyright (C) 2007-2010, 2012-2014 Gregory Nutt. All rights reserved. + * Copyright (C) 2007-2010, 2012-2014, 2016 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -56,6 +56,47 @@ * Public Functions ****************************************************************************/ +/**************************************************************************** + * Name: file_ioctl + * + * Description: + * Perform device specific operations. + * + * Parameters: + * file File structure instance + * req The ioctl command + * arg The argument of the ioctl cmd + * + * Return: + * See ioctl() below. + * + ****************************************************************************/ + +#if CONFIG_NFILE_DESCRIPTORS > 0 +int file_ioctl(FAR struct file *filep, int req, unsigned long arg) +{ + FAR struct inode *inode; + int ret; + + /* Is a driver registered? Does it support the ioctl method? */ + + inode = filep->f_inode; + if (inode && inode->u.i_ops && inode->u.i_ops->ioctl) + { + /* Yes, then let it perform the ioctl */ + + ret = (int)inode->u.i_ops->ioctl(filep, req, arg); + if (ret < 0) + { + set_errno(-ret); + return ERROR; + } + } + + return OK; +} +#endif /* CONFIG_NFILE_DESCRIPTORS > 0 */ + /**************************************************************************** * Name: ioctl/fs_ioctl * @@ -93,9 +134,8 @@ int ioctl(int fd, int req, unsigned long arg) { int errcode; #if CONFIG_NFILE_DESCRIPTORS > 0 - FAR struct file *filep; - FAR struct inode *inode; - int ret = OK; + FAR struct file *filep; + int ret = OK; /* Did we get a valid file descriptor? */ @@ -130,20 +170,9 @@ int ioctl(int fd, int req, unsigned long arg) /* Is a driver registered? Does it support the ioctl method? */ - inode = filep->f_inode; - if (inode && inode->u.i_ops && inode->u.i_ops->ioctl) - { - /* Yes, then let it perform the ioctl */ - - ret = (int)inode->u.i_ops->ioctl(filep, req, arg); - if (ret < 0) - { - errcode = -ret; - goto errout; - } - } - - return ret; + return file_ioctl(filep, req, arg); +#else + errcode = ENOTTY; #endif errout: diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h index f1bc737ffe..f383c4ac06 100644 --- a/include/nuttx/fs/fs.h +++ b/include/nuttx/fs/fs.h @@ -895,6 +895,26 @@ off_t file_seek(FAR struct file *filep, off_t offset, int whence); int file_fsync(FAR struct file *filep); #endif +/**************************************************************************** + * Name: file_ioctl + * + * Description: + * Perform device specific operations. + * + * Parameters: + * file File structure instance + * req The ioctl command + * arg The argument of the ioctl cmd + * + * Return: + * See ioctl() below. + * + ****************************************************************************/ + +#if CONFIG_NFILE_DESCRIPTORS > 0 +int file_ioctl(FAR struct file *filep, int req, unsigned long arg); +#endif + /**************************************************************************** * Name: file_vfcntl *