Re: [OpenOCD-devel] Speeding up HOST to JLink debugger exchange

[Antonio B. <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...
> <//e.mail.ru/compose/?mailto=mailto%3aopenocd%2d...@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...
> <//e.mail.ru/compose/?mailto=mailto%3aopenocd%2d...@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...
> <http:///compose?To=OpenOCD%2d...@li...>
> https://lists.sourceforge.net/lists/listinfo/openocd-devel
>
>
> _______________________________________________
> OpenOCD-devel mailing list
> Ope...@li...
> <//e.mail.ru/compose/?mailto=mailto%3aOpenOCD%2d...@li...>
> https://lists.sourceforge.net/lists/listinfo/openocd-devel
>
>
>