Menu
▾
▴
Re: [OpenOCD-devel] Speeding up HOST to JLink debugger exchange
|
From: Alexander G. <de...@ma...> - 2021-03-05 14:29:21
|
It looks like impossible to pass negative value to length field intentionally. But when my size will be stored in signed variable i will get warning from compiler about implicit conversion. Note that due to integer promotion int is most popular type.
Use unsigned variable in function interface protects from about nothing.
Unsigned values should be used only when you need full amount of bits.
--
Alexander Gabitov
>Пятница, 5 марта 2021, 16:36 +03:00 от Antonio Borneo <bor...@gm...>:
>
>I think using unsigned in the function prototype highlights that the parameter don't accepts negative values.
>The compiler "should" report any wrong use.
>
>Inside the function you can keep all the arithmetics unsigned if you avoid
>A=B-C; if (A < 0) ...
>and use directly
>if (C > B) ...
>
>Antonio
>
>On Fri, Mar 5, 2021, 11:52 Alexander Gabitov < de...@ma... > wrote:
>>This is the common mistake! Unsigned types was added for another reason to standart. But than their sense was perverted and many programmers start use them for values that cant be negative. But C standart has integer promoution — all arifmetics usually done in int. Just try to change it back to unsigned (in this function) and you will need several type casts.
>>
>>
>>--
>>Alexander Gabitov
>>
>>
>>>Пятница, 5 марта 2021, 13:27 +03:00 от Antonio Borneo < bor...@gm... >:
>>>
>>>
>>>On Fri, Mar 5, 2021, 08:08 Alexander Gabitov via OpenOCD-devel < ope...@li... > wrote:
>>>>The better way — is to fix jlink_clock_data interface. Unsigned type was a bad choice. Historically it was added to standart for unsigned arithmetics only(for rolling over max value to zero).
>>>>
>>>>static void jlink_clock_data(const uint8_t *out, int out_offset,
>>>> const uint8_t *tms_out, int tms_offset,
>>>> uint8_t *in, int in_offset,
>>>> int length)
>>>
>>>Alexander,
>>>
>>>both in_offset and length carry unsigned values only; for me they should remain unsigned and the mathematics should be modified to not use negative values
>>>
>>>Regards
>>>Antonio
>>>
>>>
>>>
>>>>{
>>>> do {
>>>> int available_length = (JLINK_TAP_BUFFER_SIZE * 8) - tap_length;
>>>> if (available_length < length ||
>>>> (in && pending_scan_results_length == MAX_PENDING_SCAN_RESULTS)) {
>>>> if (jlink_flush() != ERROR_OK)
>>>> return;
>>>> available_length = JLINK_TAP_BUFFER_SIZE * 8;
>>>> }
>>>> struct pending_scan_result *pending_scan_result =
>>>> &pending_scan_results_buffer[pending_scan_results_length];
>>>> int scan_length = length > available_length ? available_length : length; ….
>>>>}
>>>>
>>>>
>>>>--
>>>>Alexander Gabitov
>>>>
>>>>
>>>>>Пятница, 5 марта 2021, 8:12 +03:00 от Alexander Gabitov via OpenOCD-devel < ope...@li... >:
>>>>>
>>>>>I’m working on API.
>>>>>
>>>>>By the way. I think there is a bug(jlink.c:2032):
>>>>> unsigned available_length = JLINK_TAP_BUFFER_SIZE - tap_length / 8;
>>>>> if (!available_length ||
>>>>> (in && pending_scan_results_length == MAX_PENDING_SCAN_RESULTS)) {
>>>>> if (jlink_flush() != ERROR_OK)
>>>>> return;
>>>>> available_length = JLINK_TAP_BUFFER_SIZE;
>>>>> }
>>>>> struct pending_scan_result *pending_scan_result =
>>>>> &pending_scan_results_buffer[pending_scan_results_length];
>>>>> unsigned scan_length = length > available_length ?
>>>>> available_length : length;
>>>>>
>>>>>Here length is amount of bits, but available_length is amount of bytes! So this expression seems to be wrong.
>>>>>
>>>>>Also during debug i found that first condition becomes true only when second clause is true — pending_scan_results_length == MAX_PENDING_SCAN_RESULTS.
>>>>>And more over condition "!available_length" — looks very strange due to formula which calculates it.
>>>>>
>>>>>I would correct it all like that:
>>>>> int available_length = (JLINK_TAP_BUFFER_SIZE * 8) - tap_length;
>>>>> if (available_length < length ||
>>>>> (in && pending_scan_results_length == MAX_PENDING_SCAN_RESULTS)) {
>>>>> if (jlink_flush() != ERROR_OK)
>>>>> return;
>>>>> available_length = JLINK_TAP_BUFFER_SIZE * 8;
>>>>> }
>>>>> struct pending_scan_result *pending_scan_result =
>>>>> &pending_scan_results_buffer[pending_scan_results_length];
>>>>> unsigned scan_length = length > available_length ?
>>>>> available_length : length;
>>>>>
>>>>>--
>>>>>Alexander Gabitov
>>>>>
>>>>>
>>>>>_______________________________________________
>>>>>OpenOCD-devel mailing list
>>>>>Ope...@li...
>>>>>https://lists.sourceforge.net/lists/listinfo/openocd-devel
>>>> _______________________________________________
>>>>OpenOCD-devel mailing list
>>>>Ope...@li...
>>>>https://lists.sourceforge.net/lists/listinfo/openocd-devel
>>
|
