Petri Savolainen(psavol) replied on github web page:
platform/linux-generic/odp_packet.c
line 24
@@ -1739,9 +1739,68 @@ void odp_packet_print(odp_packet_t pkt)
seg = odp_packet_next_seg(pkt, seg);
}
- str[len] = '\0';
+ ODP_PRINT("%s\n", str);
+}
+
+void odp_packet_print_data(odp_packet_t pkt, uint32_t offset,
+ uint32_t byte_len)
+{
+ odp_packet_hdr_t *hdr = packet_hdr(pkt);
+ uint32_t bytes_per_row = 16;
+ int num_rows = (byte_len + bytes_per_row - 1) / bytes_per_row;
+ int max_len = 256 + (3 * byte_len) + (3 * num_rows);
+ char str[max_len];
+ int len = 0;
+ int n = max_len - 1;
+ uint32_t data_len = odp_packet_len(pkt);
+ pool_t *pool = hdr->buf_hdr.pool_ptr;
+
+ len += snprintf(&str[len], n - len, "Packet\n------\n");
+ len += snprintf(&str[len], n - len,
+ " pool index %" PRIu32 "\n", pool->pool_idx);
+ len += snprintf(&str[len], n - len,
Comment:
This patter is used in many places already. Including the odp_packet_print()
function which is just above this function in the same file. I think "someone"
modifying a line also need to know what he's doing and update pre-calculated
string length. Someone may insert a bug in many ways... with or without a check
on these lines.
Anyway, the point in this patch series is to add a new API function, and
provide a simple implementation and test for that. Once the new API is merged,
the implementation maybe modified / improved many times. If this function needs
those checks, then should all other similar functions in our code base. That
code base clean up could be done as a separate patch set (once the correct
snprintf pattern has been agreed in the community).
> Ilias Apalodimas(apalos) wrote:
> This is a standard pattern for buffer overflows. This code assumes snprintf
> returns the bytes it wrote which is not always correct. If someone adds extra
> snprintf statements without increasing max_len this would lead to overflows.
>> Petri Savolainen(psavol) wrote:
>> Yes there is. This avoids interleaved output when two (or more) threads
>> print simultaneously. Output is pretty useless if lines are interleaved.
>> Also, the current packet and buffer print functions do the same.
>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>> @muvarov they do not mention any compiler errors, so they are useless in
>>> trying to understand, which errors were there.
>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>> It's never wrong to use the PRIxxx macros so that's the safest and most
>>>> portable way to go. So I'm happy to see them used here and we should try
>>>> to use them in new development that calls for these sort of conversions.
>>>>> muvarov wrote
>>>>> @lumag compilation errors. So we have comits:
>>>>> 27a05a1d61f3b7e4d5
>>>>>
>>>>> 42d0b99c453f269f
>>>>>
>>>>> d25d276c649f57021f
>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>> @muvarov could you please be more specific? What kind of problems?
>>>>>>> muvarov wrote
>>>>>>> we had some problems with that prints in the past. Arm build or -m32 do
>>>>>>> not remember exactly what it was.
>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>> My understanding is that `PRIu32/PRIu16` are not required, you can
>>>>>>>> always use `%u/%hu` instead. Only `PRIu64/PRIx64` are required,
>>>>>>>> because exact `uint64_t` definition depends on platform ABI.
>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>> Is there any reason for printing into the string buffer, rather than
>>>>>>>>> calling `ODP_PRINT` directly?
https://github.com/Linaro/odp/pull/258#discussion_r147338586
updated_at 2017-10-27 07:22:41
