On Thu, 2017-07-27 at 01:51 -0300, Gabriel Krisman Bertazi wrote:
> Commit 34a54192e1fb ("lib/igt_debugfs: Add extended helper to format
> crc to string") introduced the following compiler warning:
>
> igt_debugfs.c: In function ‘igt_crc_to_string_extended’:
> igt_debugfs.c:373:4: warning: format not a string literal, argument
> types not checked [-Wformat-nonliteral]
> i == (crc->n_words - 1) ? '\0' : delimiter);
Thanks for the fix, I wasn't even aware that the printf format you used
("%0*x%c") existed! This is definitely what I should have done. And it's
definitely better to get rid of the fixed-length allocated buffer.
> This patch addresses the warning while also preventing a possible bad
> memory access access if n_words is > 64. I have no clue why we
> care about the padding size (crc_size), but since this was added
> recently, I'd rather leave it alone.
That's because the Chamelium CRCs are actually 2 bytes, not the full 4
that the type allows, so this avoids printing heading zeros all the
time.
> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxxxx>
> ---
> lib/igt_debugfs.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 2aa335866066..ee1f0f544824 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -351,7 +351,7 @@ bool igt_check_crc_equal(const igt_crc_t *a, const
> igt_crc_t *b)
> * igt_crc_to_string_extended:
> * @crc: pipe CRC value to print
> * @delimiter: The delimiter to use between crc words
> - * @crc_size: the number of bytes to print per crc word (either 4 or
> 2)
> + * @crc_size: the number of bytes to print per crc word (between 1
> and 4)
> *
> * This function allocates a string and formats @crc into it,
> depending on
> * @delimiter and @crc_size.
> @@ -362,16 +362,19 @@ bool igt_check_crc_equal(const igt_crc_t *a,
> const igt_crc_t *b)
> char *igt_crc_to_string_extended(igt_crc_t *crc, char delimiter, int
> crc_size)
> {
> int i;
> - char *buf = calloc(128, sizeof(char));
> - const char *format[2] = { "%08x%c", "%04x%c" };
> + int len = 0;
> + int field_width = 2 * crc_size; /* Two chars per byte. */
> + char *buf = malloc((field_width+1) * crc->n_words *
> sizeof(char));
>
> if (!buf)
> return NULL;
>
> for (i = 0; i < crc->n_words; i++)
> - sprintf(buf + strlen(buf), format[crc_size == 2],
> crc->crc[i],
> - i == (crc->n_words - 1) ? '\0' : delimiter);
> + len += sprintf(buf + len, "%0*x%c", field_width,
> + crc->crc[i], delimiter);
>
> + /* Eat the last delimiter */
> + buf[len - 1] = '\0';
> return buf;
> }
>
--
Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxxxxxx>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
