| Message ID | 20210914121129.51451-8-joevt@shaw.ca |
|---|---|
| State | New |
| Headers | show |
| Series | [01/11] edid-decode: add more example EDIDs | expand |
On 14/09/2021 14:11, joevt wrote: > hex_block should not return without printing a newline (which occurs when the length is zero). This causes a missing newline after "Application Version: 1" with cta_hdr10plus for an EDID I have. > Any place that calls hex_block will have the same problem if it's possible for the length to be zero. > > In other words, a hex_block needs to have a linefeed even if it has zero length, because the caller assumes it will go to the next line as it does when the hex block is not zero length. > > Signed-off-by: Joe van Tunen <joevt@shaw.ca> > --- > edid-decode.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/edid-decode.cpp b/edid-decode.cpp > index 2316abc..6aa93fb 100644 > --- a/edid-decode.cpp > +++ b/edid-decode.cpp > @@ -698,8 +698,10 @@ void hex_block(const char *prefix, const unsigned char *x, > { > unsigned i, j; > > - if (!length) > + if (!length) { > + printf("\n"); > return; > + } Hmm, with this change I get this: edid-decode -c Digital/Acer/ACR0282/B12D637C1F12 with the linuxhw database: Vendor-Specific Data Block (HDMI), OUI 00-0C-03: Source physical address: 2.0.0.0 Unknown CTA-861 tag 0x00, length 0 Unknown CTA-861 tag 0x00, length 0 Unknown CTA-861 tag 0x00, length 0 Unknown CTA-861 tag 0x00, length 0 Unknown CTA-861 tag 0x00, length 0 Unknown CTA-861 tag 0x00, length 0 Unknown CTA-861 tag 0x00, length 0 That looks pretty ugly. Regards, Hans > > for (i = 0; i < length; i += step) { > unsigned len = min(step, length - i); >
Yes, that's ugly. I will do a search for the EDID that prompted me to make this change. Maybe it's not a problem anymore.
...
Seems like the problem I had was fixed in cta_hdr10plus. I'll do more checking and testing with other calls to hex_block.
On 2021-09-15, 3:10 AM, "Hans Verkuil" <hverkuil@xs4all.nl> wrote:
On 14/09/2021 14:11, joevt wrote:
> hex_block should not return without printing a newline (which occurs when the length is zero). This causes a missing newline after "Application Version: 1" with cta_hdr10plus for an EDID I have.
> Any place that calls hex_block will have the same problem if it's possible for the length to be zero.
>
> In other words, a hex_block needs to have a linefeed even if it has zero length, because the caller assumes it will go to the next line as it does when the hex block is not zero length.
>
> Signed-off-by: Joe van Tunen <joevt@shaw.ca>
> ---
> edid-decode.cpp | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/edid-decode.cpp b/edid-decode.cpp
> index 2316abc..6aa93fb 100644
> --- a/edid-decode.cpp
> +++ b/edid-decode.cpp
> @@ -698,8 +698,10 @@ void hex_block(const char *prefix, const unsigned char *x,
> {
> unsigned i, j;
>
> - if (!length)
> + if (!length) {
> + printf("\n");
> return;
> + }
Hmm, with this change I get this:
edid-decode -c Digital/Acer/ACR0282/B12D637C1F12 with the linuxhw database:
Vendor-Specific Data Block (HDMI), OUI 00-0C-03:
Source physical address: 2.0.0.0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
That looks pretty ugly.
Regards,
Hans
>
> for (i = 0; i < length; i += step) {
> unsigned len = min(step, length - i);
>
I found no place that requires this change to hex_block.
This patch should be ignored.
I did find one call to hex_block that should be modified:
cta_hdr10plus outputs hex on the same line as the "Application Version: %u".
It either needs to always output a linefeed before calling hex_block like this:
printf(" Application Version: %u\n", x[0]);
hex_block(" ", x + 1, length - 1);
Or it needs to set step to the same value as length, like this:
printf(" Application Version: %u", x[0]);
if (length > 1)
hex_block(" ", x + 1, length - 1, true, length - 1);
else
printf("\n");
Those are probably the only acceptable ways to call hex_block (first is multi-line or no-line, second is one-line)
It should also probably check the length:
if (length == 0) {
fail("Empty Data Block with length %u.\n", length);
return;
}
On 2021-09-15, 8:43 AM, "Joe van Tunen" <joevt@shaw.ca> wrote:
Yes, that's ugly. I will do a search for the EDID that prompted me to make this change. Maybe it's not a problem anymore.
...
Seems like the problem I had was fixed in cta_hdr10plus. I'll do more checking and testing with other calls to hex_block.
On 2021-09-15, 3:10 AM, "Hans Verkuil" <hverkuil@xs4all.nl> wrote:
On 14/09/2021 14:11, joevt wrote:
> hex_block should not return without printing a newline (which occurs when the length is zero). This causes a missing newline after "Application Version: 1" with cta_hdr10plus for an EDID I have.
> Any place that calls hex_block will have the same problem if it's possible for the length to be zero.
>
> In other words, a hex_block needs to have a linefeed even if it has zero length, because the caller assumes it will go to the next line as it does when the hex block is not zero length.
>
> Signed-off-by: Joe van Tunen <joevt@shaw.ca>
> ---
> edid-decode.cpp | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/edid-decode.cpp b/edid-decode.cpp
> index 2316abc..6aa93fb 100644
> --- a/edid-decode.cpp
> +++ b/edid-decode.cpp
> @@ -698,8 +698,10 @@ void hex_block(const char *prefix, const unsigned char *x,
> {
> unsigned i, j;
>
> - if (!length)
> + if (!length) {
> + printf("\n");
> return;
> + }
Hmm, with this change I get this:
edid-decode -c Digital/Acer/ACR0282/B12D637C1F12 with the linuxhw database:
Vendor-Specific Data Block (HDMI), OUI 00-0C-03:
Source physical address: 2.0.0.0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
Unknown CTA-861 tag 0x00, length 0
That looks pretty ugly.
Regards,
Hans
>
> for (i = 0; i < length; i += step) {
> unsigned len = min(step, length - i);
>
diff --git a/edid-decode.cpp b/edid-decode.cpp index 2316abc..6aa93fb 100644 --- a/edid-decode.cpp +++ b/edid-decode.cpp @@ -698,8 +698,10 @@ void hex_block(const char *prefix, const unsigned char *x, { unsigned i, j; - if (!length) + if (!length) { + printf("\n"); return; + } for (i = 0; i < length; i += step) { unsigned len = min(step, length - i);
hex_block should not return without printing a newline (which occurs when the length is zero). This causes a missing newline after "Application Version: 1" with cta_hdr10plus for an EDID I have. Any place that calls hex_block will have the same problem if it's possible for the length to be zero. In other words, a hex_block needs to have a linefeed even if it has zero length, because the caller assumes it will go to the next line as it does when the hex block is not zero length. Signed-off-by: Joe van Tunen <joevt@shaw.ca> --- edid-decode.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)