[ofa-general] Re: [PATCH] osm: source and destination strings overlap when using sprintf()

[Yevgeny Kliteynik]

Michael S. Tsirkin wrote:
>> Quoting Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>:
>> Subject: [PATCH] osm: source and destination strings overlap when using sprintf()
>>
>> Hi Hal,
>>
>> Fixing a problematic usage of sprintf() in osm_helper.c:
>>
>> When using sprintf(), source and destination strings should
>> not overlap, otherwise the function behavior is undefined.
>>
>> Please apply to ofed_1_2 and to master.
>>
>> Thanks.
>>
>> -- Yevgeny
>>
>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>> ---
>> osm/opensm/osm_helper.c |   47 
>> ++++++++++++++++++++++++++++++++++-------------
>> 1 files changed, 34 insertions(+), 13 deletions(-)
> 
> .... skip ...
> 	
>>     for (i = 0; i < 32; i++)
>> -      sprintf( buf_line2,"%s 0x%01X |",
>> -               buf_line2, p_vla_tbl->vl_entry[i].weight);
>> +    {
>> +      sprintf( tmp_buf_line," 0x%01X |",
>> +               p_vla_tbl->vl_entry[i].weight);
>> +      strcat( buf_line2, tmp_buf_line );
>> +    }
>>     osm_log( p_log, log_level,
>>              "VlArb dump:\n"
>>              "\t\t\tport_guid...........0x%016" PRIx64 "\n"
> 
> These tmp-bufs are quite ugly, and bloat the code up.
> Since you seem to do a strcat which does an anyway, how about, for example:
> 
> -      sprintf( buf_line1,"%s 0x%01x |",
> -               buf_line1, p_vla_tbl->vl_entry[i].vl);
> +      sprintf( buf_line1 + strlen(buf_line1)," 0x%01x |",
> +               p_vla_tbl->vl_entry[i].vl);
> 
> and so on in all the other places?

Agree.
I'll send a new patch later.

-- Yevgeny