xiaoxiang781216 commented on code in PR #8755:
URL: https://github.com/apache/nuttx/pull/8755#discussion_r1133041771
##########
drivers/syslog/ramlog.c:
##########
@@ -286,84 +296,42 @@ static int ramlog_addchar(FAR struct ramlog_dev_s *priv,
char ch)
flags = enter_critical_section();
- /* Calculate the write index AFTER the next byte is written */
-
- nexthead = priv->rl_head + 1;
- if (nexthead >= priv->rl_bufsize)
- {
- nexthead = 0;
- }
+ /* Calculate the size in the buffer */
- /* Would the next write overflow the circular buffer? */
+ space = (int)priv->rl_head - (int)priv->rl_tail >= 0 ?
Review Comment:
```suggestion
space = priv->rl_head >= priv->rl_tail ?
```
##########
drivers/syslog/ramlog.c:
##########
@@ -286,84 +296,42 @@ static int ramlog_addchar(FAR struct ramlog_dev_s *priv,
char ch)
flags = enter_critical_section();
- /* Calculate the write index AFTER the next byte is written */
-
- nexthead = priv->rl_head + 1;
- if (nexthead >= priv->rl_bufsize)
- {
- nexthead = 0;
- }
+ /* Calculate the size in the buffer */
- /* Would the next write overflow the circular buffer? */
+ space = (int)priv->rl_head - (int)priv->rl_tail >= 0 ?
+ priv->rl_bufsize - (priv->rl_head - priv->rl_tail) - 1:
+ priv->rl_tail - priv->rl_head - 1;
- if (nexthead == priv->rl_tail)
+ if (len > space)
Review Comment:
need handle len > rl_bufsize correctly
##########
drivers/syslog/ramlog.c:
##########
@@ -286,84 +296,42 @@ static int ramlog_addchar(FAR struct ramlog_dev_s *priv,
char ch)
flags = enter_critical_section();
- /* Calculate the write index AFTER the next byte is written */
-
- nexthead = priv->rl_head + 1;
- if (nexthead >= priv->rl_bufsize)
- {
- nexthead = 0;
- }
+ /* Calculate the size in the buffer */
- /* Would the next write overflow the circular buffer? */
+ space = (int)priv->rl_head - (int)priv->rl_tail >= 0 ?
+ priv->rl_bufsize - (priv->rl_head - priv->rl_tail) - 1:
+ priv->rl_tail - priv->rl_head - 1;
- if (nexthead == priv->rl_tail)
+ if (len > space)
{
#ifdef CONFIG_RAMLOG_OVERWRITE
- /* Yes... Overwrite with the latest log in the circular buffer */
-
- priv->rl_buffer[priv->rl_tail] = '\0';
- priv->rl_tail += 1;
- if (priv->rl_tail >= priv->rl_bufsize)
- {
- priv->rl_tail = 0;
- }
+ priv->rl_tail = (priv->rl_tail + len - space) % priv->rl_bufsize;
#else
- /* Yes... Return an indication that nothing was saved in the buffer. */
-
- leave_critical_section(flags);
- return -EBUSY;
+ len = space;
#endif
}
- /* No... copy the byte and re-enable interrupts */
-
- priv->rl_buffer[priv->rl_head] = ch;
- priv->rl_head = nexthead;
-
- leave_critical_section(flags);
- return OK;
-}
-
-/****************************************************************************
- * Name: ramlog_addbuf
- ****************************************************************************/
-
-static ssize_t ramlog_addbuf(FAR struct ramlog_dev_s *priv,
- FAR const char *buffer, size_t len)
-{
- int readers_waken;
- ssize_t nwritten;
- char ch;
- int ret;
-
- ret = nxmutex_lock(&priv->rl_lock);
- if (ret < 0)
+ head = priv->rl_head;
+ if (priv->rl_head + len > priv->rl_bufsize)
{
- return ret;
+ remain = priv->rl_bufsize - priv->rl_head;
+ priv->rl_head = priv->rl_head + len - priv->rl_bufsize;
}
-
- for (nwritten = 0; (size_t)nwritten < len; nwritten++)
+ else
{
- /* Get the next character to output */
-
- ch = buffer[nwritten];
-
- /* Then output the character */
+ remain = len;
+ priv->rl_head = priv->rl_head + len;
+ }
- ret = ramlog_addchar(priv, ch);
- if (ret < 0)
- {
- /* The buffer is full and nothing was saved. The remaining
- * data to be written is dropped on the floor.
- */
+ /* No... copy the byte and re-enable interrupts */
- break;
- }
- }
+ leave_critical_section(flags);
Review Comment:
it's wrong leave critical section before copy buffer.
##########
drivers/syslog/ramlog.c:
##########
@@ -286,84 +296,42 @@ static int ramlog_addchar(FAR struct ramlog_dev_s *priv,
char ch)
flags = enter_critical_section();
- /* Calculate the write index AFTER the next byte is written */
-
- nexthead = priv->rl_head + 1;
- if (nexthead >= priv->rl_bufsize)
- {
- nexthead = 0;
- }
+ /* Calculate the size in the buffer */
Review Comment:
```suggestion
/* Calculate the space in the buffer */
```
##########
drivers/syslog/ramlog.c:
##########
@@ -286,84 +296,42 @@ static int ramlog_addchar(FAR struct ramlog_dev_s *priv,
char ch)
flags = enter_critical_section();
- /* Calculate the write index AFTER the next byte is written */
-
- nexthead = priv->rl_head + 1;
- if (nexthead >= priv->rl_bufsize)
- {
- nexthead = 0;
- }
+ /* Calculate the size in the buffer */
- /* Would the next write overflow the circular buffer? */
+ space = (int)priv->rl_head - (int)priv->rl_tail >= 0 ?
+ priv->rl_bufsize - (priv->rl_head - priv->rl_tail) - 1:
+ priv->rl_tail - priv->rl_head - 1;
- if (nexthead == priv->rl_tail)
+ if (len > space)
{
#ifdef CONFIG_RAMLOG_OVERWRITE
- /* Yes... Overwrite with the latest log in the circular buffer */
-
- priv->rl_buffer[priv->rl_tail] = '\0';
- priv->rl_tail += 1;
- if (priv->rl_tail >= priv->rl_bufsize)
- {
- priv->rl_tail = 0;
- }
+ priv->rl_tail = (priv->rl_tail + len - space) % priv->rl_bufsize;
#else
- /* Yes... Return an indication that nothing was saved in the buffer. */
-
- leave_critical_section(flags);
- return -EBUSY;
+ len = space;
#endif
}
- /* No... copy the byte and re-enable interrupts */
-
- priv->rl_buffer[priv->rl_head] = ch;
- priv->rl_head = nexthead;
-
- leave_critical_section(flags);
- return OK;
-}
-
-/****************************************************************************
- * Name: ramlog_addbuf
- ****************************************************************************/
-
-static ssize_t ramlog_addbuf(FAR struct ramlog_dev_s *priv,
- FAR const char *buffer, size_t len)
-{
- int readers_waken;
- ssize_t nwritten;
- char ch;
- int ret;
-
- ret = nxmutex_lock(&priv->rl_lock);
- if (ret < 0)
+ head = priv->rl_head;
+ if (priv->rl_head + len > priv->rl_bufsize)
Review Comment:
if (priv->rl_head + len >= priv->rl_bufsize)
{
remain = priv->rl_bufsize - priv->rl_head;
memcpy(priv->rl_buffer + priv->rl_head, buffer, remain);
memcpy(priv->rl_buffer, buffer + remain, len - remain);
priv->rl_head = len - remain;
}
else
{
memcpy(priv->rl_buffer + priv->rl_head, buffer, len);
priv->rl_head += len;
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
