You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/11/17 06:11:41 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request #2315: Add common circular buffer management
Donny9 opened a new pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315
## Summary
Add common circular buffer management. And the sensor driver model and rc driver model use it to manage intermediate buffer.
## Impact
Make it easier to manage the circular buffer in device driver model.
## Testing
daily test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#issuecomment-730107995
@v01d is this PR ready for merge now?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525720509
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
No, you should follow the NuttX naming conventions as specified in the Coding Standard: https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#funcname
Personal preference is irrelevant.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525723915
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
Review comment:
I think "if(!circ)" and DEBUGASSERT(circ) don't have to exist together, we need return error value rather than assert this place. How do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525750183
##########
File path: drivers/sensors/sensor.c
##########
@@ -801,9 +654,9 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
/* Initialize sensor buffer */
- ret = sensor_buffer_create(&upper->buffer,
- lower->type, lower->buffer_size);
- if (ret)
+ bytes = ROUNDUP(lower->buffer_size, g_sensor_info[lower->type].esize);
Review comment:
When fetch is NULL, the lower->buffer_size is zero, and ROUNDUP(lower->buffer_size,...) also is zero, so circ_buffer_init will not alloc buffer space for sensor using fetch api. This ensures code consistency between not fetch and use fetch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#issuecomment-728731889
@v01d please take a look the new common circle buffer implementation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526245248
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,474 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required while only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can be
+ * provided by caller because sometimes the creation of buffer
+ * is special or needs to preallocated, eg: DMA buffer.
+ * If NULL, a buffer of the given size will be allocated.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!base || bytes);
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!circ->external);
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ circ->head = circ->tail = 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+
+ if (!circ->external)
+ {
+ kmm_free(circ->base);
+ }
+
+ memset(circ, 0, sizeof(*circ));
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->size;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->head - circ->tail;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(dst || !bytes);
+
+ bytes = circ_buf_peek(circ, dst, bytes);
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_skip
+ *
+ * Description:
+ * Skip data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - Number of bytes to skip.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the skip data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_skip(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ size_t len;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_write
+ *
+ * Description:
+ * Write data to the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * src - The data to be added.
+ * bytes - Number of bytes to be added.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the write data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_write(FAR struct circ_buf_s *circ,
+ FAR const void *src, size_t bytes)
+{
+ size_t space;
+ size_t off;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(src || !bytes);
Review comment:
Ok, we can leave it, as long as bytes == 0 does the right thing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526111350
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
Review comment:
I don't really follow the data handling after the resize. It would seem it could contain partial data. What is the intent here?
Does it make sense to resized to something not a multiple of the data struct size?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525752816
##########
File path: drivers/sensors/sensor.c
##########
@@ -867,6 +720,6 @@ void sensor_unregister(FAR struct sensor_lowerhalf_s *lower, int devno)
nxsem_destroy(&upper->exclsem);
nxsem_destroy(&upper->buffersem);
- sensor_buffer_release(upper->buffer);
+ circ_buf_uninit(&upper->buffer);
Review comment:
okay, i will optimize code. Thank you .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525733189
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
Review comment:
yes, i do
##########
File path: drivers/sensors/sensor.c
##########
@@ -77,7 +68,7 @@ struct sensor_upperhalf_s
FAR struct pollfd *fds[CONFIG_SENSORS_NPOLLWAITERS];
FAR struct sensor_lowerhalf_s *lower; /* the handle of lower half driver */
- FAR struct sensor_buffer_s *buffer; /* The circualr buffer of sensor device */
+ struct circ_buf_s buffer; /* The circualr buffer of sensor device */
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526129189
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,474 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required while only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can be
+ * provided by caller because sometimes the creation of buffer
+ * is special or needs to preallocated, eg: DMA buffer.
+ * If NULL, a buffer of the given size will be allocated.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!base || bytes);
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!circ->external);
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ circ->head = circ->tail = 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+
+ if (!circ->external)
+ {
+ kmm_free(circ->base);
+ }
+
+ memset(circ, 0, sizeof(*circ));
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->size;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->head - circ->tail;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(dst || !bytes);
+
+ bytes = circ_buf_peek(circ, dst, bytes);
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_skip
+ *
+ * Description:
+ * Skip data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - Number of bytes to skip.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the skip data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_skip(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ size_t len;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_write
+ *
+ * Description:
+ * Write data to the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * src - The data to be added.
+ * bytes - Number of bytes to be added.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the write data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_write(FAR struct circ_buf_s *circ,
+ FAR const void *src, size_t bytes)
+{
+ size_t space;
+ size_t off;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(src || !bytes);
Review comment:
I agree that it is OK to model a queue of size 0, but it does not make much sense to allow read/write of size 0 as valid. Also, why would you do anything to the queue when using fetch? Isn't it exactly the case there when the queue is not used?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525748835
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
But I think circ_buf_s already follow the guide. Anyway, what's name you prefer? circ_buf, ringbuffer or cbuffer. We can change to what you want.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526246847
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
Review comment:
> > I don't really follow the data handling after the resize. It would seem it could contain partial data. What is the intent here?
>
> many motion sensor contain a big ram(around several KB) and then can buffer several seconds data before interrupt(or wake) the host. Many wearable devices utilize this feature like this:
>
> 1. Command the sensor enter the low power mode(enable the buffer) when the screan is off
>
> 2. The sensor gather the data until the buffer ram is full and than wake up the host
>
> 3. The sensor driver read all data in batch(note: the internal buffer is enlarge in step 1 by circ_buf_resize):
> https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L550-L554
>
> 4. When the userspace read out the data and empty the circle buffer to some threshold, sensor driver will call circ_buf_resize again to free the unused memory:
> https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
>
>
> The keypoint is that all unread data in the circle buffer must be kept unchange regardless the circle buffer become bigger or smaller. Since the unread data may split into two discrete part(beginning and ending), we have to move/merge it in the new circle buffer. kmm_realloc make the move/merge become very hard in this case.
Thanks I understand now the intention.
>
> > Does it make sense to resized to something not a multiple of the data struct size?
>
> First, circ_buf is byte orient and don't have the data unit concept, it's the caller responsibility to ensure read/write/resize with the right data unit. If you look at sensor.c carefully, all related place pass the multiple of the data struct size.
> Second, if the caller shrink circ_buf smaller than the unread data, yes, the oldest data will lose just like when the buffer is full(since we want to drop the oldest data, we can't use kmm_realloc here). It's also the caller responsibility to shrink circ_buf with the right size or in the right time if he don't want to lose the data like the sensor driver:
> https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
Right, I'm confusing both PRs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525161235
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
Review comment:
```suggestion
* base - A pointer to circular buffer's internal buffer. It can be provided
* by caller because sometimes the creation of buffer
```
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
Review comment:
```suggestion
* If NULL, a buffer of the given size will be allocated
```
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
Review comment:
```suggestion
* is special or needs to be preallocated, eg: DMA buffer.
```
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
Review comment:
wouldn't it be better to use kmm_realloc for all this?
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ circ->head = circ->tail = 0;
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ if (circ && !circ->external)
+ {
+ kmm_free(circ->base);
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ return circ->size;
+ }
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ return circ->head - circ->tail;
+ }
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
Review comment:
```suggestion
* With only one concurrent reader and one concurrent writer,
```
same thing every where else
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
Review comment:
maybe a debugassert to check if base is non-NULL then bytes cannot be zero?
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ circ->head = circ->tail = 0;
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ if (circ && !circ->external)
+ {
+ kmm_free(circ->base);
Review comment:
you should also reset all internal data of circ to catch any errors of using uninitialized buffer again
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ circ->head = circ->tail = 0;
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ if (circ && !circ->external)
+ {
+ kmm_free(circ->base);
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ return circ->size;
+ }
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ return circ->head - circ->tail;
+ }
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ if (!circ || !dst)
+ {
+ return -EINVAL;
+ }
+
+ bytes = circ_buf_peek(circ, dst, bytes);
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_skip
+ *
+ * Description:
+ * Skip data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - Number of bytes to skip.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the skip data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_skip(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ size_t len;
+
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ len = circ_buf_used(circ);
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_write
+ *
+ * Description:
+ * Write data to the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * src - The data to be added.
+ * bytes - Number of bytes to be added.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the write data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_write(FAR struct circ_buf_s *circ,
+ FAR const void *src, size_t bytes)
+{
+ size_t space;
+ size_t off;
+
+ if (!circ || !src)
+ {
+ return -EINVAL;
+ }
+
+ space = circ_buf_space(circ);
+ off = circ->head % circ->size;
+ if (bytes > space)
+ {
+ bytes = space;
+ }
+
+ space = circ->size - off;
+ if (bytes < space)
+ {
+ space = bytes;
+ }
+
+ memcpy(circ->base + off, src, space);
+ memcpy(circ->base, src + space, bytes - space);
+ circ->head += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_overwrite
+ *
+ * Description:
+ * Write data to the circular buffer. It can overwrite old data when
+ * circular buffer don't have enough space to store data.
+ *
+ * Note:
+ * Usage circ_buf_overwrite () is dangerous. It should be only called
Review comment:
```suggestion
* Using circ_buf_overwrite () is dangerous. It should be only called
```
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
Review comment:
is there a good reason to allow circ == NULL as a valid input everywhere (I mean, not consider it a programming error)? I think it would be best to add DEBUGASSERT(circ) to every function. This could catch programming errors.
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
Review comment:
```suggestion
/* Note about locking: There is no locking required while only one reader
```
?
##########
File path: drivers/sensors/sensor.c
##########
@@ -77,7 +68,7 @@ struct sensor_upperhalf_s
FAR struct pollfd *fds[CONFIG_SENSORS_NPOLLWAITERS];
FAR struct sensor_lowerhalf_s *lower; /* the handle of lower half driver */
- FAR struct sensor_buffer_s *buffer; /* The circualr buffer of sensor device */
+ struct circ_buf_s buffer; /* The circualr buffer of sensor device */
Review comment:
```suggestion
struct circ_buf_s buffer; /* The circular buffer of sensor device */
```
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ circ->head = circ->tail = 0;
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ if (circ && !circ->external)
+ {
+ kmm_free(circ->base);
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ return circ->size;
+ }
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ return circ->head - circ->tail;
+ }
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ if (!circ || !dst)
+ {
+ return -EINVAL;
+ }
+
+ bytes = circ_buf_peek(circ, dst, bytes);
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_skip
+ *
+ * Description:
+ * Skip data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - Number of bytes to skip.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the skip data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_skip(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ size_t len;
+
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ len = circ_buf_used(circ);
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_write
+ *
+ * Description:
+ * Write data to the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * src - The data to be added.
+ * bytes - Number of bytes to be added.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the write data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_write(FAR struct circ_buf_s *circ,
+ FAR const void *src, size_t bytes)
+{
+ size_t space;
+ size_t off;
+
+ if (!circ || !src)
+ {
+ return -EINVAL;
+ }
+
+ space = circ_buf_space(circ);
+ off = circ->head % circ->size;
+ if (bytes > space)
+ {
+ bytes = space;
+ }
+
+ space = circ->size - off;
+ if (bytes < space)
+ {
+ space = bytes;
+ }
+
+ memcpy(circ->base + off, src, space);
+ memcpy(circ->base, src + space, bytes - space);
+ circ->head += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_overwrite
+ *
+ * Description:
+ * Write data to the circular buffer. It can overwrite old data when
+ * circular buffer don't have enough space to store data.
Review comment:
```suggestion
* circular buffer doesn't have enough space to store data.
```
##########
File path: drivers/sensors/sensor.c
##########
@@ -801,9 +654,9 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
/* Initialize sensor buffer */
- ret = sensor_buffer_create(&upper->buffer,
- lower->type, lower->buffer_size);
- if (ret)
+ bytes = ROUNDUP(lower->buffer_size, g_sensor_info[lower->type].esize);
Review comment:
Same concern as in previous PR: when using only fetch, won't this allocate unneeded memory? Why is it needed if fetch will write directly to user buffer?
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
Review comment:
Add some one-line comments to members, if possible
##########
File path: drivers/sensors/sensor.c
##########
@@ -867,6 +720,6 @@ void sensor_unregister(FAR struct sensor_lowerhalf_s *lower, int devno)
nxsem_destroy(&upper->exclsem);
nxsem_destroy(&upper->buffersem);
- sensor_buffer_release(upper->buffer);
+ circ_buf_uninit(&upper->buffer);
Review comment:
Same comment as in previous PR: this memory should be allocated/allocated on first open()/ last close().
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526124239
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,474 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required while only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can be
+ * provided by caller because sometimes the creation of buffer
+ * is special or needs to preallocated, eg: DMA buffer.
+ * If NULL, a buffer of the given size will be allocated.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!base || bytes);
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!circ->external);
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ circ->head = circ->tail = 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+
+ if (!circ->external)
+ {
+ kmm_free(circ->base);
+ }
+
+ memset(circ, 0, sizeof(*circ));
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->size;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->head - circ->tail;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(dst || !bytes);
+
+ bytes = circ_buf_peek(circ, dst, bytes);
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_skip
+ *
+ * Description:
+ * Skip data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - Number of bytes to skip.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the skip data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_skip(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ size_t len;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_write
+ *
+ * Description:
+ * Write data to the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * src - The data to be added.
+ * bytes - Number of bytes to be added.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the write data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_write(FAR struct circ_buf_s *circ,
+ FAR const void *src, size_t bytes)
+{
+ size_t space;
+ size_t off;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(src || !bytes);
Review comment:
we provide struct circ_buf_s instance in device driver, but sometimes init it with bytes = 0, so we need to support bytes is zero. ex: Use fetch in sensor driver when intermedinate buffer's bytes is 0.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525781574
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
>
>
> But I think circ_buf_s already follow the guide. Anyway, what's name you prefer? circ_buf, ringbuffer or cbuffer. We can change to what you want.
There is a more complete description of naming conventions for OS internal functions in this wiki page: https://cwiki.apache.org/confluence/display/NUTTX/Naming+of+OS+Internal+Functions
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526112082
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,474 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required while only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can be
+ * provided by caller because sometimes the creation of buffer
+ * is special or needs to preallocated, eg: DMA buffer.
+ * If NULL, a buffer of the given size will be allocated.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!base || bytes);
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!circ->external);
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ circ->head = circ->tail = 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+
+ if (!circ->external)
+ {
+ kmm_free(circ->base);
+ }
+
+ memset(circ, 0, sizeof(*circ));
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->size;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->head - circ->tail;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(dst || !bytes);
Review comment:
why would you read zero bytes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#issuecomment-730114639
It is good to go for me. Only remaining issue was the naming but for me it is not that important, mostly a preference.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526112321
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,474 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required while only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can be
+ * provided by caller because sometimes the creation of buffer
+ * is special or needs to preallocated, eg: DMA buffer.
+ * If NULL, a buffer of the given size will be allocated.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!base || bytes);
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!circ->external);
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ circ->head = circ->tail = 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+
+ if (!circ->external)
+ {
+ kmm_free(circ->base);
+ }
+
+ memset(circ, 0, sizeof(*circ));
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->size;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->head - circ->tail;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(dst || !bytes);
+
+ bytes = circ_buf_peek(circ, dst, bytes);
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_skip
+ *
+ * Description:
+ * Skip data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - Number of bytes to skip.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the skip data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_skip(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ size_t len;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_write
+ *
+ * Description:
+ * Write data to the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * src - The data to be added.
+ * bytes - Number of bytes to be added.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the write data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_write(FAR struct circ_buf_s *circ,
+ FAR const void *src, size_t bytes)
+{
+ size_t space;
+ size_t off;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(src || !bytes);
Review comment:
same, why allow zero bytes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r527554190
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
okay. Done!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526242485
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,474 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required while only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can be
+ * provided by caller because sometimes the creation of buffer
+ * is special or needs to preallocated, eg: DMA buffer.
+ * If NULL, a buffer of the given size will be allocated.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!base || bytes);
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(!circ->external);
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ circ->head = circ->tail = 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+
+ if (!circ->external)
+ {
+ kmm_free(circ->base);
+ }
+
+ memset(circ, 0, sizeof(*circ));
+}
+
+/****************************************************************************
+ * Name: circ_buf_size
+ *
+ * Description:
+ * Return size of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_size(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->size;
+}
+
+/****************************************************************************
+ * Name: circ_buf_used
+ *
+ * Description:
+ * Return the used bytes of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_used(FAR struct circ_buf_s *circ)
+{
+ DEBUGASSERT(circ);
+ return circ->head - circ->tail;
+}
+
+/****************************************************************************
+ * Name: circ_buf_space
+ *
+ * Description:
+ * Return the remaing space of the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+size_t circ_buf_space(FAR struct circ_buf_s *circ)
+{
+ return circ_buf_size(circ) - circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_empty
+ *
+ * Description:
+ * Return true if the circular buffer is empty.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_empty(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_used(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_is_full
+ *
+ * Description:
+ * Return true if the circular buffer is full.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+bool circ_buf_is_full(FAR struct circ_buf_s *circ)
+{
+ return !circ_buf_space(circ);
+}
+
+/****************************************************************************
+ * Name: circ_buf_peek
+ *
+ * Description:
+ * Get data form the circular buffer without removing
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the peek data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_peek(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ size_t len;
+ size_t off;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+ off = circ->tail % circ->size;
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ len = circ->size - off;
+ if (bytes < len)
+ {
+ len = bytes;
+ }
+
+ memcpy(dst, circ->base + off, len);
+ memcpy(dst + len, circ->base, bytes - len);
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_read
+ *
+ * Description:
+ * Get data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * dst - Address where to store the data.
+ * bytes - Number of bytes to get.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the read data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_read(FAR struct circ_buf_s *circ,
+ FAR void *dst, size_t bytes)
+{
+ DEBUGASSERT(circ);
+ DEBUGASSERT(dst || !bytes);
+
+ bytes = circ_buf_peek(circ, dst, bytes);
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_skip
+ *
+ * Description:
+ * Skip data form the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - Number of bytes to skip.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the skip data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_skip(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ size_t len;
+
+ DEBUGASSERT(circ);
+
+ len = circ_buf_used(circ);
+
+ if (bytes > len)
+ {
+ bytes = len;
+ }
+
+ circ->tail += bytes;
+
+ return bytes;
+}
+
+/****************************************************************************
+ * Name: circ_buf_write
+ *
+ * Description:
+ * Write data to the circular buffer.
+ *
+ * Note :
+ * That with only one concurrent reader and one concurrent writer,
+ * you don't need extra locking to use these api.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * src - The data to be added.
+ * bytes - Number of bytes to be added.
+ *
+ * Returned Value:
+ * The bytes of get data is returned if the write data is successful;
+ * A negated errno value is returned on any failure.
+ ****************************************************************************/
+
+ssize_t circ_buf_write(FAR struct circ_buf_s *circ,
+ FAR const void *src, size_t bytes)
+{
+ size_t space;
+ size_t off;
+
+ DEBUGASSERT(circ);
+ DEBUGASSERT(src || !bytes);
Review comment:
But it isn't harm to allow zero size, right? and it also follow the standard file read/write convention:
ret = read(fd, buf, 0);
ret is 0 not -1. The same convention could let the user forward to circ_buf_s function directly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525901566
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
Review comment:
okay, i do.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525725361
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526108626
##########
File path: drivers/sensors/sensor.c
##########
@@ -801,9 +654,9 @@ int sensor_register(FAR struct sensor_lowerhalf_s *lower, int devno)
/* Initialize sensor buffer */
- ret = sensor_buffer_create(&upper->buffer,
- lower->type, lower->buffer_size);
- if (ret)
+ bytes = ROUNDUP(lower->buffer_size, g_sensor_info[lower->type].esize);
Review comment:
Ok, I was confused with the purpose of ROUNDUP. I assumed it would return `g_sensor_info[lower->type].esize` in that case. If it returns 0 it is OK.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526233482
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
Review comment:
> I don't really follow the data handling after the resize. It would seem it could contain partial data. What is the intent here?
many motion sensor contain a big ram(around several KB) and then can buffer several seconds data before interrupt(or wake) the host. Many wearable devices utilize this feature like this:
1. Command the sensor enter the low power mode(enable the buffer) when the screan is off
2. The sensor gather the data until the buffer ram is full and than wake up the host
3. The sensor driver read all data in batch(note: the internal buffer is enlarge in step 1 by circ_buf_resize):
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L550-L554
4. When the userspace read out the data and empty the circle buffer to some threshold, sensor driver will call circ_buf_resize again to free the unused memory:
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
The keypoint is that all unread data in the circle buffer must be kept unchange regardless the circle buffer become bigger or smaller. Since the unread data may split into two discrete part(beginning and ending), we have to move/merge it in the new circle buffer. kmm_realloc make the move/merge become very hard in this case.
> Does it make sense to resized to something not a multiple of the data struct size?
First, circ_buf is byte orient and don't have the data unit concept, it's the caller responsibility to ensure read/write/resize with the right data unit. If you look at sensor.c carefully, all related place pass the multiple of the data struct size.
Second, if the caller shrink circ_buf smaller than the unread data, yes, the oldest data will lose just like when the buffer is full(since we want to drop the oldest data, we can't use kmm_realloc here). It's also the caller responsibility to shrink circ_buf with the right size or in the right time if he don't want to lose the data like the sensor driver:
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525720509
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
No, you should follow the NuttX naming conventions as specified in the Coding Standard: https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#funcname
Personal preference and Linux naming conventions are irrelevant.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525733484
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
Review comment:
I mean to replace all ifs with the DEBUGASSERT. Error checking in runtime for a value which should not normally occur unnecessarily complicates using the API. circ == NULL should not normally occur so I think it is better to replace all these checks with DEBUGASSERT().
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526233482
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
Review comment:
> I don't really follow the data handling after the resize. It would seem it could contain partial data. What is the intent here?
many motion sensor contain a big ram(around several KB) and then can buffer several seconds data before interrupt(or wake) the host. Many wearable devices utilize this feature like this:
1. Command the sensor enter the low power mode(enable the buffer) when the screan is off
2. The sensor gather the data until the buffer ram is full and than wake up the host
3. The sensor driver read all data in batch(note: the internal buffer is enlarge in step 1 by circ_buf_resize):
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L550-L554
4. When the userspace read out the data and empty the circle buffer to some threshold, sensor driver will call circ_buf_resize again to free the unused memory:
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
The keypoint is that all unread data in the circle buffer must be kept unchange regardless the circle buffer become bigger or smaller. Since the unread data may split into two discrete part(beginning and ending), we have to move/merge it in the new circle buffer. kmm_realloc make the move/merge become very hard in this case.
> Does it make sense to resized to something not a multiple of the data struct size?
First, circ_buf is byte orient and don't have the data unit concept, it's the caller responsibility to ensure read/write/resize with the right data unit. If you look at sensor.c carefully, all related place pass the multiple of the data struct size.
Second, if the caller shrink circ_buf smaller than the unread data, yes, the oldest data will lose just like when the buffer is full. It's also the caller responsibility to shrink circ_buf with the right size or in the right time if he don't want to lose the data like the sensor driver:
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526621135
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
@patacongo basically, circ_buf API follow the suggested style:
```
<subsystem>_<verb>_<object>[_<units>]
```
circ_buf is <sbusystem>, init... is <verb>, the difference is that API is simple enough to remove the object.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525181300
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
The init would follow.
`create` - `destroy`
`allocate` - `free`
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
Would `destroy` or `free` be a better name and give the notion of de-allocation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525725585
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
Review comment:
done
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525683869
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
>
>
> Looks good, just few minor comments everywhere. Mostly concerned about uneeded memory use.
>
> I'm not a fan of the naming with an underscore and abbreviation (circ_buf), would probably be better if cbuffer or ringbuffer would be used. But this is just aesthetic comment.
I just refer to linux naming convention, it may be better. https://github.com/torvalds/linux/blob/master/include/linux/circ_buf.h <smile>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525956903
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
Review comment:
Okay, but i don't want mm_realloc because:
1. Realloc is the equivalent of malloc when you're small to a huge size, ex: we into sensor batch mode from not-batch mode.
2. The buffer is circular buffer, the reduced buffer may not be the one we want, it might be in the middle.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525726668
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
Review comment:
yes, i need check this condition
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525195649
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
>
>
> The init would follow.
> `create` - `destroy`
> `allocate` - `free`
Hello, Davids, I define struct circ_buf_s instance in other device driver and call circ_buf_init to initialize, it feels better to init on the semantic.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r525726944
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526233482
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
Review comment:
> I don't really follow the data handling after the resize. It would seem it could contain partial data. What is the intent here?
many motion sensor contain a big ram(around several KB) and then can buffer several seconds data before interrupt(or wake) the host. Many wearable devices utilize this feature like this:
1. Command the sensor enter the low power mode(enable the buffer) when the screan is off
2. The sensor gather the data until the buffer ram is full and than wake up the host
3. The sensor driver read all data in batch(note: the internal buffer is enlarge in step 1 by circ_buf_resize):
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L550-L554
4. When the userspace read out the data to shrink the circle buffer to some threshold, sensor driver will call circ_buf_resize again to free the unused memory:
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
The keypoint is that all unread data in the circle buffer must be kept unchange regardless the circle buffer become bigger or smaller. Since the unread data may split into two discrete part(beginning and ending), we have to move/merge it in the new circle buffer.
> Does it make sense to resized to something not a multiple of the data struct size?
First, circ_buf is byte orient and don't have the data unit concept, it's the caller responsibility to ensure read/write/resize with the right data unit. If you look at sensor.c carefully, all related place pass the multiple of the data struct size.
Second, if the caller shrink circ_buf smaller than the unread data, yes, the oldest data will lose just like when the buffer is full. It's also the caller responsibility to shrink circ_buf with the right size or in the right time if he don't want to lose the data like the sensor driver:
https://github.com/apache/incubator-nuttx/blob/master/drivers/sensors/sensor.c#L464-L470
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526055620
##########
File path: mm/circ_buf/circ_buf.c
##########
@@ -0,0 +1,496 @@
+/****************************************************************************
+ * mm/circ_buf/circ_buf.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circ_buf.h>
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ, FAR void *base, size_t bytes)
+{
+ if (!circ)
+ {
+ return -EINVAL;
+ }
+
+ circ->external = !!base;
+
+ if (!base && bytes)
+ {
+ base = kmm_malloc(bytes);
+ if (!base)
+ {
+ return -ENOMEM;
+ }
+ }
+
+ circ->base = base;
+ circ->size = bytes;
+ circ->head = 0;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes)
+{
+ FAR void *tmp;
+ size_t len;
+
+ if (!circ || circ->external)
+ {
+ return -EINVAL;
+ }
+
+ tmp = kmm_malloc(bytes);
+ if (!tmp)
+ {
+ return -ENOMEM;
+ }
+
+ len = circ_buf_used(circ);
+ if (bytes < len)
+ {
+ circ_buf_skip(circ, len - bytes);
+ len = bytes;
+ }
+
+ circ_buf_read(circ, tmp, len);
+
+ kmm_free(circ->base);
+
+ circ->base = tmp;
+ circ->size = bytes;
+ circ->head = len;
+ circ->tail = 0;
+
+ return 0;
+}
+
+/****************************************************************************
+ * Name: circ_buf_reset
+ *
+ * Description:
+ * Remove the entire circular buffer content.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_reset(FAR struct circ_buf_s *circ)
+{
+ if (circ)
+ {
+ circ->head = circ->tail = 0;
+ }
+}
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ)
+{
+ if (circ && !circ->external)
+ {
+ kmm_free(circ->base);
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2315: Add common circular buffer management
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2315:
URL: https://github.com/apache/incubator-nuttx/pull/2315#discussion_r526873771
##########
File path: include/nuttx/mm/circ_buf.h
##########
@@ -0,0 +1,308 @@
+/****************************************************************************
+ * include/nuttx/mm/circ_buf.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_MM_CIRC_BUF_H
+#define __INCLUDE_NUTTX_MM_CIRC_BUF_H
+
+/* Note about locking: There is no locking required until only one reader
+ * and one writer is using the circular buffer.
+ * For multiple writer and one reader there is only a need to lock the
+ * writer. And vice versa for only one writer and multiple reader there is
+ * only a need to lock the reader.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdbool.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* This structure describes circular buffer */
+
+struct circ_buf_s
+{
+ FAR void *base;
+ size_t size;
+ size_t head;
+ size_t tail;
+ bool external;
+};
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+/****************************************************************************
+ * Name: circ_buf_init
+ *
+ * Description:
+ * Initialize a circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * base - A pointer to circular buffer's internal buffer. It can provide
+ * by called context because sometimes the creation of buffer
+ * is speical or preallocated, eg: DMA buffer.
+ * It also can alloc by this api if base is NULL.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_init(FAR struct circ_buf_s *circ,
+ FAR void *base, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_resize
+ *
+ * Description:
+ * Resize a circular buffer (change buffer size).
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ * bytes - The size of the internal buffer.
+ *
+ * Returned Value:
+ * Zero on success; A negated errno value is returned on any failure.
+ *
+ ****************************************************************************/
+
+int circ_buf_resize(FAR struct circ_buf_s *circ, size_t bytes);
+
+/****************************************************************************
+ * Name: circ_buf_uninit
+ *
+ * Description:
+ * Free the circular buffer.
+ *
+ * Input Parameters:
+ * circ - Address of the circular buffer to be used.
+ ****************************************************************************/
+
+void circ_buf_uninit(FAR struct circ_buf_s *circ);
Review comment:
I think what "breaks the rule" is the underscore. IMHO it would look better with any name which does not include an underscore (eg: circbuf, cbuffer, ringbuf, etc.). Anyway, I'll let you decide on it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org