[AHTOXA 14]

2 часа назад, razrab83 сказал:

int s = rw - rp;

Если обе переменных в правой части выражения volatile, то компилятор будет выдавать предупреждение.

Но в целом я согласен с вашими замечаниями, в плане стиля.

С другой стороны, вы когда-нибудь видели, чтобы кто-то кого-то смог переубедить? :D

[Arlleex 131]

3 часа назад, razrab83 сказал:

Вот это что такое?...

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

 

3 часа назад, razrab83 сказал:

и что с того что volatile? cравните...

Сравнил. На Ваш пример нормальный компилятор выдаст предупреждение и будет прав.

 

P.S. Размер буфера #define RB_BUF_SIZE (100 + 1). Если Вы имеете ввиду, что макрос FreeSpace() возвращает значение на 1 больше реального, то будете правы и это учитывается в функциях. Более того, макросы я эти переписал немного так, что возвращают они сразу нужное число, ибо так, все-таки, корректнее.

 

1 час назад, AHTOXA сказал:

С другой стороны, вы когда-нибудь видели, чтобы кто-то кого-то смог переубедить? :D

То то же

[jcxz 184]

3 часа назад, razrab83 сказал:

Вот это что такое? s = RB_BUF_SIZE*((u32)i  >> 31) + i; ? сравнение "<0"!!! Если там всё кристально чисто, зачем вы предлагаете нормальный человеческий вариант if ((i -= RB->RP) < 0) i += RB_BUF_SIZE;

Я это предложил просто как более оптимальный в данном конкретном случае. Ничего индусского в исходном варианте Arlleex нет. Можно и так делать.

3 часа назад, razrab83 сказал:

зачем из однострочного кода делать 2-х строчный? Ведь короткий код и читать быстрее/понятнее! Теперь про volatile.... пусть rp и rw volatile - и что, какое преимущество даст 2-х строчный код?

1-первых: Сначала попробуйте скормить такое компилятору и посмотреть как он вас при этом назовёт.

2-вторых: Повторю ещё раз то, что говорил выше: Почитайте литературу про потокобезопасное программирование. Если это изучите, тогда поймёте почему без volatile там никак нельзя. А пока только видно, что Вы в этой теме совсем не разбираетесь.

1 час назад, AHTOXA сказал:

Если обе переменных в правой части выражения volatile, то компилятор будет выдавать предупреждение.

Не только злой компилятор тут виноват. Он не зря выдаёт предупреждение. volatile используется тогда, когда порядок доступа к переменным имеет значение. И здесь имеет место как раз тот случай - кольцевой буфер используемый для интерфейса между разными процессами/ISR.

Компилятор просто говорит: "Я могу скомпилить код так, что он станет неработоспособным".

[Arlleex 131]

26 минут назад, jcxz сказал:

Это Вы мне? Если это правда, тогда у Вас в коде где-то ошибка.

Это я @razrab83.

 

Распишу вкратце, как я задумывал работу буфера.

Допустим, есть буфер длиной N элементов. Есть индексы WP (позиция следующей записи) и RP (позиция текущего чтения) этого буфера. Эти индексы бегают в диапазоне от 0 до (N - 1).

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

Значит, нужно вводить какое-то ограничение. Равенство указателей будет означать, что буфер полностью свободен. Но тогда придется пожертвовать одним элементом буфера, чтобы правильно интерпретировать полностью забитый буфер. Это тот случай, когда голова догнала свой хвост с левой стороны. Если я запишу последний N-элемент, позиция головы сдвинется и станет равна позиции хвоста; то есть получится, что буфер (судя по указателям) пуст! Поэтому FreeSpace() всегда возвращает значение, на 1 меньшее N.

N у меня это RB_BUF_SIZE. А RB_BUF_SIZE это (100 + 1), как бы показывая, что 100 элементов там, которые действительно будут использоваться, а 1 лишь для правильной интерпретации положения указателей. В примере, который я давал выше, FreeSpace() у меня возвращала значение, на 1 больше реального, поэтому в коде я делал s32 FLen; FreeSpace(FLen); if(--Flen > ...). Сейчас я переписал этот макрос и он сразу выдает мне именно реальное количество свободных элементов.

[razrab83 17]

1

 

Изменено 21 декабря, 2019 пользователем razrab83

[jcxz 184]

21 минуту назад, Arlleex сказал:

Это я @razrab83.

Я уже откорректировал тот пост. Я перепутал - думал Вы пишете про другой макрос.

21 минуту назад, Arlleex сказал:

Допустим, есть буфер длиной N элементов. Есть индексы WP (позиция следующей записи) и RP (позиция текущего чтения) этого буфера. Эти индексы бегают в диапазоне от 0 до (N - 1).

Вот именно - несколькими постами выше я Вам как раз написал, что у Вас индексы не в диапазоне 0...(N-1), а в диапазоне 1...N. Я писал там как переделать.

21 минуту назад, Arlleex сказал:

Значит, нужно вводить какое-то ограничение. Равенство указателей будет означать, что буфер полностью свободен. Но тогда придется пожертвовать одним элементом буфера, чтобы правильно интерпретировать полностью забитый буфер.

Это всё самой собой разумеется. Это следует из логики работы кольцевого буфера.

[razrab83 17]

22 часа назад, jcxz сказал:

1-первых: Сначала попробуйте скормить такое компилятору и посмотреть как он вас при этом назовёт. 

скормил. -Wall -Wextra. arm-none-eabi- выдал  Build Finished. 0 errors, 0 warnings.

arm-linux-gnueabihf- выдал Build Finished. 0 errors, 0 warnings.

22 часа назад, jcxz сказал:

Ничего индусского в исходном варианте Arlleex нет. Можно и так делать. 

Вы почитайте - что такое "индусский код", потом оперируйте этим понятием.

 

22 часа назад, jcxz сказал:

поймёте почему без volatile там никак нельзя.

зачем вы приписываете другим то, что они не говорили? Вам уже сделали замечание "Отучаемся говорить за всех". Процитируйте где я говорил, что  volatile не нужен. Я этого ни говорил. Вы книжку хорошего тона почитайте, потом регистрируйтесь на форумах.

 

22 часа назад, Arlleex сказал:

Размер буфера #define RB_BUF_SIZE (100 + 1). Если Вы имеете ввиду, что макрос FreeSpace()...

нет. просто если буфер был бы равен 128 или 64 (кратное 2^x), то занулять переполнение в for можно так

us32 j;...//хотя, можно и s32 оставить, в вашем случае индекс не большое значение, в j ни когда переполнения не произойдет. 
Buf[i] = RB->Buf[j & (RB_BUF_SIZE-1)]

в вашем коде выбор индекса - куча операций... "-", ">>", "&" и "=". А тут всего одна - "&".

[jcxz 184]

5 минут назад, razrab83 сказал:

зачем вы приписываете другим то, что они не говорили? Пиздобол чтоли? Вам уже сделали замечание "Отучаемся говорить за всех". Процитируйте где я говорил, что  volatile не нужен. Я этого ни говорил. Вы книжку хорошего тона почитайте, потом регистрируйтесь на форумах.

Блин, ещё один...  Откуда вас только выпускают?

[Arlleex 131]

11 минут назад, razrab83 сказал:

нет. просто если буфер был бы равен 128 или 64...

Разумеется, о занулении по маске я в курсе.

Но эта реализация как раз для случая произвольного размера буфера.

 

P.S. Вижу обсуждение перерастает в другое русло. Предлагаю на этом и закончить.

Разговоры о вкусах и личных предпочтениях можно продолжать до бесконечности и каждый по-своему будет прав, а для кого-то нет.

[Integro 0]

Блин.... все пропустил)
 

Подытоживая, я бы назвал основные признаки плохочитаемого кода:
 - нужно гадать назначение переменных\функций (отсутствие самодокументированности)
 - функция(макрос) не явно выполнят какие либо действия над 3им объектом
 - этот код хочется переписать, что даже сделал уважаемый @jcxz

@Arlleex , Ваш код рабочий, но извините, назвать его понятным нельзя.

Мы все, по сути, изо дня в день, перекидываем байты из периферии "А" в периферию "Б", вопрос только в том будет ли поддерживаться ваш код вами (или другим разработчиком) в будущем и можно ли данный код без каких либо модификаций перенести в следующий проект.

[razrab83 17]

В 21.12.2019 в 13:39, Arlleex сказал:

На Ваш пример нормальный компилятор выдаст предупреждение и будет прав.

Хочу нормальный компилятор!!! ))... не, реально.... не ради тролинга, а чтоб через общение и обмен опытом развиваться... 

проверил двумя gcc, проверил кейлом и иаром - ворнинга нет.

1)Arlleex какой компилятор вам выдал ворнинг?

2)Arlleex и AHTOXA (и все остальные) - почему компилятор даст ворнинг?

 

ps. jcxz пожалуйста, не отвечайте на эти вопросы, не стоит вам тратить своё время.

Изменено 23 декабря, 2019 пользователем razrab83

[AHTOXA 14]

39 минут назад, razrab83 сказал:

2)Arlleex и AHTOXA (и все остальные) - почему компилятор даст ворнинг? 

Потому что порядок доступа к volatile-переменным не определён. Компилятор имеет право вычислять выражение в правой части в любом порядке. Об этом он и будет предупреждать.

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

[AHTOXA 14]

Ага, вот, нашёл. Оказывается, это IAR так ругался: тынц.

[Arlleex 131]

6 часов назад, razrab83 сказал:

1)Arlleex какой компилятор вам выдал ворнинг?

IAR. Keil в этом плане несколько слабоват...

 

6 часов назад, razrab83 сказал:

2)Arlleex и AHTOXA (и все остальные) - почему компилятор даст ворнинг?

Впрочем, АНТОХА опередил.

 

6 часов назад, razrab83 сказал:

проверил двумя gcc, проверил кейлом и иаром - ворнинга нет.

int main( void )
{
  volatile char a = 10;
  volatile char b = 20;
  
  char c = a - b;
  
  return 0;
}

даст warning. А Вы как проверяли?

[razrab83 17]

12 часов назад, Arlleex сказал:

даст warning. А Вы как проверяли?

я проверял так

 volatile int a = 100;
 volatile int b = 300;
int main( void )
{
int y = 4; //неиспользуемая переменная, на неё должен быть ворнинг, индикатор вывода ворнингов
int c = a - b;
return c;
}

проверял в реальных проектах.

проверил ваш код - нет ворнинга. побочный эффект спора обсуждения, нашел ресурс, там 100500 онлайн компиляторов gcc -  проверяй любым. Ни на одном компиляторе не получил ворнинга, даже на 8-ми битных. Ни на С, ни на С++.

16 часов назад, AHTOXA сказал:

Оказывается, это IAR так ругался: тынц.

В иаре, в арме не дает ругонь. А может быть ...... @еееееее..... ПОЛУЧИЛ этот ворнинг!!! Создал с нуля проект в иаре - ворнинг появился. Почему нет в текущих проектах на ИАР и в gcc - не понятно.... ну да ладно....

в тынц пример от IAR

intAnalogSample[internalChannelSelect] = ADC[internalChannelSelect];

в этом примере конечно ошибка, к гадалке не ходи не нужен ворниг

 

 

и тут с "?" и с volitile будет ошибка. Не нужно ждать ворнинг, тут чутьлине "явная" ошибка.

s = RB->WP < RB->RP  ? RB->WP - RB->RP  + RB_BUF_SIZE: RB->WP - RB->RP;

а вот с if ошибки не будет (при определённых условиях).

 

 

в случае int c = a - b (см выше) - криминала нет, даже если есть ворнинг. (при условии, что int 4 байта и это 32-х битный проц). Какая разница в каком порядке а и бэ? Ну обратится сначала к "а", потом к "b" - или наоборот?

18 часов назад, AHTOXA сказал:

Компилятор имеет право вычислять выражение в правой части в любом порядке. Об этом он и будет предупреждать.

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

 

а вот если это 8-ми битник, то тут ой.... тут с volatile улетим.... но.... вот правильный код т.е. "правильный код" для 8-ми битника

  volatile int a = 10;
  volatile int b = 20;
int main( void )
{
  int temp;
  temp = a;
  temp -= b;
    
  return temp;
}

вот код, на который нет ворнинга. Но... тут нет атомарности. temp = a;  будет выполняться за 2 или 4 цикла (для 16-ти и 32-х битного int). Т.е. во время копирования из "а" в "temp" может "а" измениться. Держите пулю в ногу. И отсутствие ворнинга тут не поможет. Тут другими методами защиту многопоточности нужно обеспечивать.

 

 

Изменено 24 декабря, 2019 пользователем razrab83