Hi Thomas, On 29.01.20 18:44, Thomas Schwinge wrote:
>> + size_t len = sizeof hsa_context.driver_version_s; >> + int printed = snprintf (hsa_context.driver_version_s, len, >> + "HSA Runtime %hu.%hu", (unsigned short int)major, >> + (unsigned short int)minor); >> + if (printed >= len) >> + GCN_WARNING ("HSA runtime version string was truncated." >> + "Version %hu.%hu is too long.", (unsigned short int)major, >> + (unsigned short int)minor); > > (Can it actually happen that 'snprintf' returns 'printed > len' -- > meaning that it's written into random memory? I thought 'snprintf' has a > hard stop at 'len'? Or does this indicate the amount of memory it > would've written? I should re-read the manpage at some point...) ;-) > Yes, "printed > len" can happen. Seems that I have chosen a bad variable name. "actual_len" (of the formatted string that should have been written - excluding the terminating '\0') would have been more appropriate. > For 'printed = len' does or doesn't 'snprintf' store the terminating > 'NUL' character, or do we manually have to set: > > hsa_context.driver_version_s[len - 1] = '\0'; > > ... in that case? No, in this case, the printed string is missing the last character, but the terminating '\0' has been written. Consider: #include <stdio.h> int main () { char s[] = "foo"; char buf[3]; // buf is too short to hold terminating '\0' int actual_len = snprintf (buf, 3, "%s", s); printf ("buf: %s\n", buf); printf ("actual_len: %d\n", actual_len); } Output: buf: fo actual_len: 3 > >> @@ -3410,15 +3432,19 @@ GOMP_OFFLOAD_init_device (int n) > >> - char buf[64]; >> status = hsa_fns.hsa_agent_get_info_fn (agent->id, HSA_AGENT_INFO_NAME, >> - &buf); >> + &agent->name); >> if (status != HSA_STATUS_SUCCESS) >> return hsa_error ("Error querying the name of the agent", status); > > (That's of course pre-existing, but) this looks like a dangerous API, > given that 'hsa_agent_get_info_fn' doesn't know 'sizeof agent->name' (or > 'sizeof buf' before)... The API documentation (cf. https://rocm-documentation.readthedocs.io/en/latest/ROCm_API_References/ROCr-API.html) states that "the type of this attribute is a NUL-terminated char[64]". But, right, should this ever change, we might not notice it. Best regards, Frederik