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: us...@infra.apache.org
