[mdmitry 0]

С какой целью в usrlib.h в описании шаблона класса ring_buffer

template<typename T, uint16_t Size, typename S = uint8_t>
    class ring_buffer

тип S uint8_t? На мой взгляд заложена потенциальная опасность при использовании умолчания шаблона.

Код:

#include <iostream>
#include "usrlib.h"
using namespace std;

#define RFBUF_SIZE 	1024
#define DTBUF_SIZE	255

usr::ring_buffer<uint8_t, RFBUF_SIZE, uint16_t> full_description;
usr::ring_buffer<uint8_t, RFBUF_SIZE> short_description;

uint8_t databuf[DTBUF_SIZE];

int main()
{
for(uint8_t i=0; i < DTBUF_SIZE; i++)
	databuf[i] = i;

full_description.write(&databuf[0], DTBUF_SIZE - 8);
short_description.write(&databuf[0], DTBUF_SIZE - 8);

cout << dec;
cout << "full_description: get_count = " << full_description.get_count() << " get_free_size = " << full_description.get_free_size() << endl;
cout << "short_description: get_count = " << short_description.get_count() << " get_free_size = " << short_description.get_free_size() << endl;

full_description.write(&databuf[0], DTBUF_SIZE - 8);
short_description.write(&databuf[0], DTBUF_SIZE - 8);

cout << "full_description: get_count = " << full_description.get_count() << " get_free_size = " << full_description.get_free_size() << endl;
cout << "short_description: get_count = " << short_description.get_count() << " get_free_size = " << short_description.get_free_size() << endl;

return 0;
}

 

Вывод:

full_description: get_count = 247 get_free_size = 777
short_description: get_count = ч get_free_size =     
full_description: get_count = 494 get_free_size = 530
short_description: get_count = о get_free_size = 

 

При включенных предупреждениях компилятора потенциальная опасность видна сразу.

Вопрос: из каких соображений в шаблоне по умолчанию тип S не совпадает с типом Size?

Спасибо.

[dxp 33]

Вопрос: из каких соображений в шаблоне по умолчанию тип S не совпадает с типом Size?

Ноги растут из "древности", из первых версий, когда основными процессорами были AVR и MSP430, актуально было экономить каждый байт, а буферов таких размеров и быть не могло - там памяти столько не было. Сегодня, наверное, самое правильное замутить тут traits. Подумаем на эту тему. Спасибо!

[mdmitry 0]

В функции

template<typename T, uint16_t Size, typename S>
T usr::ring_buffer<T, Size, S>::pop_item_back()
{

    if(Last == 0)
        Last = Size - 1;
    else
        --Last;

    Count--;
    return Buf[Last];;
}

наверное, не требуется в последней строке (где return) два символа ;? Это в версии из trunk.

[2gav 0]

Простите, что пишу в древнюю ветку, просто здесь не организовано разбиение на темы по модулям.

 

А вопрос у меня такой.

 

версия userlib.h: Version: v5.1.0

 

почему методы <write> и <read>, класса <ring_buffer> такие не единообразные?

 

bool usr::ring_buffer<T, Size, S>::write(const T* data, const S cnt)
{
    if( cnt > (Size - Count) )
        return false;

    for(S i = 0; i < cnt; i++)
        push_item(*(data++));

    return true;
}
void usr::ring_buffer<T, Size, S>::read(T* data, const S cnt)
{
    S nItems = cnt <= Count ? cnt : Count;

    for(S i = 0; i < nItems; i++)
        data[i] = pop_item();
}

т.е. что я имею ввиду:

 

<read>: контролирует кол-во данных и если их меньше, чем запрашивают, то считывает столько сколько есть, однако, при этом не возвращает кол-во считанных данных.

<write>: ничего не делает, если кол-во свободного места меньше, чем хотят записать, а не записывает столько сколько может (аналогично read), с последующим возвратом кол-во записанных данных.

 

Мне кажется, что более качественно было бы, если read считывал MIN(Count, cnt) и возвращал бы это кол-во данных, а write аналогично, писал бы MIN(Size-Count, cnt) и возвращал бы кол-во записанных данных.

Ведь тут <Count> - это не размер в байтах, а именно кол-во единиц элементов типа <T>.

 

Используется данный класс <ring_buffer> в компоненте <channel>, где тоже странная реализация у методов:

S OS::channel<T, Size, S>::write_isr(const T* data, const S count)
{
    TCritSect cs;

    const S free = pool.get_free_size();
    S qty = free < count ? free : count;
    pool.write(data, qty);
    resume_all_isr(ConsumersProcessMap);
    return qty;
}
S OS::channel<T, Size, S>::read_isr(T* const data, const S max_size)
{
    TCritSect cs;

    const S avail = pool.get_count();
    S count = avail < max_size ? avail : max_size;
    pool.read(data, count);
    resume_all_isr(ProducersProcessMap);
    return count;
}

т.е. метод write_isr пользуется методом <pool.write>, но не контролирует возвращаемый им признак.

А read_isr, зачем-то контролирует <pool.Count>, обрезает считываемый размер и при этом не смотрит на возращаемое значение pool.read, когда сам метод <ring_buffer.read> делает все тоже самое.. и по делу.

 

Куда компактней было бы написать так

(в случае, если бы методы ring_buffer: read/write были бы универсальными):

S OS::channel<T, Size, S>::write_isr(const T* data, const S count)
{
    TCritSect cs;

    S qty = pool.write(data, count);
    resume_all_isr(ConsumersProcessMap);
    return qty;
}
S OS::channel<T, Size, S>::read_isr(T* const data, const S max_size)
{
    TCritSect cs;

    S count = pool.read(data, max_size);
    resume_all_isr(ProducersProcessMap);
    return count;
}

Раз компонент <channel> использует свой (под себя написанный) класс <ring_buffer>, то и перезакладываться на контроль чужого (для класса channel) <Count> при вызове read/write нет никакой необходимости.

 

Я не прав?

 

[Сергей Борщ 119]

Мне кажется, что более качественно было бы, если read считывал MIN(Count, cnt) и возвращал бы это кол-во данных, а write аналогично, писал бы MIN(Size-Count, cnt) и возвращал бы кол-во записанных данных.

Ваше предложение разумное. Я сам буквально пару недель назад пришел к такому же выводу. Но мы не можем радикально менять поведение OS::channel для совместимости со старыми проектами. В то же время ничего не мешает нам написать новый класс. Давайте все вместе придумаем ему понятное название и вместе напишем реализацию. Мне пока кроме stream и new_channel ничего в голову не приходит. C new_channel закладываем себе грабли - через сколько-то лет надо будет убирать слово new_ и заменять на что-то другое. advanced_channel?

[dxp 33]

почему методы <write> и <read>, класса <ring_buffer> такие не единообразные?

 

bool usr::ring_buffer<T, Size, S>::write(const T* data, const S cnt)
{
    if( cnt > (Size - Count) )
        return false;

    for(S i = 0; i < cnt; i++)
        push_item(*(data++));

    return true;
}
void usr::ring_buffer<T, Size, S>::read(T* data, const S cnt)
{
    S nItems = cnt <= Count ? cnt : Count;

    for(S i = 0; i < nItems; i++)
        data[i] = pop_item();
}

т.е. что я имею ввиду:

 

<read>: контролирует кол-во данных и если их меньше, чем запрашивают, то считывает столько сколько есть, однако, при этом не возвращает кол-во считанных данных.

<write>: ничего не делает, если кол-во свободного места меньше, чем хотят записать, а не записывает столько сколько может (аналогично read), с последующим возвратом кол-во записанных данных.

Такая асимметрия возникает из-за характера применения функций чтения и записи. При чтении ожидающий процесс ничего не знает о количестве данных в буфере, если специально не запрашивать. Поэтому как только что-то там есть, это что-то отдаётся "читателю". У "писателя" ситуация несколько иная - как правило при записи пишется некий законченный блок данных (пакет) и нет никакого смысла дробить его из-за появления в буфере сколько-нибудь места. Если делать симметрично, то некоторых случаях будет возникать либо торможение на стороне приёмника (если отдавать только запрошенное количество данных), либо будет возникать дополнительная обязательная нагрузка на пользовательский код - вместо того, чтобы один раз вызвать запрос за запись в канал (а класс буфера там спроектирован именно как основа для сервисного класса ОС channel), пользователю придётся отслеживать порции записи и в цикле дописывать.

 

Вот такие соображения. Тут можно спорить, что лучше, но вот такие рассуждения были 15 лет назад, исходя их тех реалий (в первую очередь возможностей элементной базы и обозримого класса задач).

 

Мне кажется, что более качественно было бы, если read считывал MIN(Count, cnt) и возвращал бы это кол-во данных, а write аналогично, писал бы MIN(Size-Count, cnt) и возвращал бы кол-во записанных данных.

Ведь тут <Count> - это не размер в байтах, а именно кол-во единиц элементов типа <T>.

См. выше.

 

Используется данный класс <ring_buffer> в компоненте <channel>, где тоже странная реализация у методов:

S OS::channel<T, Size, S>::write_isr(const T* data, const S count)
{
    TCritSect cs;

    const S free = pool.get_free_size();
    S qty = free < count ? free : count;
    pool.write(data, qty);
    resume_all_isr(ConsumersProcessMap);
    return qty;
}
S OS::channel<T, Size, S>::read_isr(T* const data, const S max_size)
{
    TCritSect cs;

    const S avail = pool.get_count();
    S count = avail < max_size ? avail : max_size;
    pool.read(data, count);
    resume_all_isr(ProducersProcessMap);
    return count;
}

т.е. метод write_isr пользуется методом <pool.write>, но не контролирует возвращаемый им признак.

А read_isr, зачем-то контролирует <pool.Count>, обрезает считываемый размер и при этом не смотрит на возращаемое значение pool.read, когда сам метод <ring_buffer.read> делает все тоже самое.. и по делу.

Использование сервиса channel в обработчиках прерываний в общем случае небезопасно, т.к. в случае нехватки ресурса будет произведена попытка встать на ожидание (ресурса) и отдать управление ядру, чего в прерывании делать нельзя. Поэтому позже (кстати, совсем не так давно) были добавлены функции *_isr, которые не приводят к фатальным последствиям, т.к. проверяют наличие ресурса и только после этого обращаются к нему. По нюнасам реализации прокомментировать не могу, автор этого добавления не я.

 

Можно ли тут что-то отрулить, не берусь с ходу судить: с одной изменения могут сломать что-то у пользователя, с другой - те, что были внесены совсем недавно (read/write_isr) вряд ли "обросли историей" и если логика использования не меняется, то реализацию можно и подшаманить безопасно.

 

 

Ваше предложение разумное. Я сам буквально пару недель назад пришел к такому же выводу. Но мы не можем радикально менять поведение OS::channel для совместимости со старыми проектами. В то же время ничего не мешает нам написать новый класс. Давайте все вместе придумаем ему понятное название и вместе напишем реализацию. Мне пока кроме stream и new_channel ничего в голову не приходит. C new_channel закладываем себе грабли - через сколько-то лет надо будет убирать слово new_ и заменять на что-то другое. advanced_channel?

Да, трогать старые channel::read/write, полагаю, нельзя - обязательно сломается пользовательский код. И если хочется своего, то таки да, такая возможность есть штатно - написать свою реализацию в виде расширения. По названию могу предложить - pipe. Смысл и уровень абстракции такой же как и у channel в данном контексте, а имя совершенно другое и даже короче. :)