mdmitry 0 26 сентября, 2013 Опубликовано 26 сентября, 2013 · Жалоба С какой целью в 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 56 27 сентября, 2013 Опубликовано 27 сентября, 2013 · Жалоба Вопрос: из каких соображений в шаблоне по умолчанию тип S не совпадает с типом Size? Ноги растут из "древности", из первых версий, когда основными процессорами были AVR и MSP430, актуально было экономить каждый байт, а буферов таких размеров и быть не могло - там памяти столько не было. Сегодня, наверное, самое правильное замутить тут traits. Подумаем на эту тему. Спасибо! Цитата Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
mdmitry 0 27 сентября, 2013 Опубликовано 27 сентября, 2013 · Жалоба В функции 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 26 июля, 2018 Опубликовано 26 июля, 2018 · Жалоба Простите, что пишу в древнюю ветку, просто здесь не организовано разбиение на темы по модулям. А вопрос у меня такой. версия 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 нет никакой необходимости. Я не прав? Цитата Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
Сергей Борщ 134 28 июля, 2018 Опубликовано 28 июля, 2018 · Жалоба Мне кажется, что более качественно было бы, если read считывал MIN(Count, cnt) и возвращал бы это кол-во данных, а write аналогично, писал бы MIN(Size-Count, cnt) и возвращал бы кол-во записанных данных.Ваше предложение разумное. Я сам буквально пару недель назад пришел к такому же выводу. Но мы не можем радикально менять поведение OS::channel для совместимости со старыми проектами. В то же время ничего не мешает нам написать новый класс. Давайте все вместе придумаем ему понятное название и вместе напишем реализацию. Мне пока кроме stream и new_channel ничего в голову не приходит. C new_channel закладываем себе грабли - через сколько-то лет надо будет убирать слово new_ и заменять на что-то другое. advanced_channel? Цитата Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться
dxp 56 30 июля, 2018 Опубликовано 30 июля, 2018 · Жалоба почему методы <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 в данном контексте, а имя совершенно другое и даже короче. :) Цитата Поделиться сообщением Ссылка на сообщение Поделиться на другие сайты Поделиться