[openib-general] [PATCH] diags/saquery: fix node_desc.description as string usages

[Sasha Khapyorsky]

On 13:44 Mon 30 Oct     , Michael S. Tsirkin wrote:
> Quoting r. Sasha Khapyorsky <sashak at voltaire.com>:
> > Subject: [PATCH] diags/saquery: fix node_desc.description as string usages
> > 
> > 
> > node_desc.description buffer is received from the network and should
> > not be NULL-terminated. In such cases using it as regular string in
> > functions like strcmp() or printf() leads to segmentation faults.
> > This patch fixes such usages.
> > 
> > Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> > ---
> >  diags/src/saquery.c |   22 ++++++++++++++++------
> >  1 files changed, 16 insertions(+), 6 deletions(-)
> > 
> > diff --git a/diags/src/saquery.c b/diags/src/saquery.c
> > index 5b4a85e..f5b23fd 100644
> > --- a/diags/src/saquery.c
> > +++ b/diags/src/saquery.c
> > @@ -90,17 +90,21 @@ static void
> >  print_node_desc(ib_node_record_t *node_record)
> >  {
> >  	ib_node_info_t *p_ni = &(node_record->node_info);
> > +	ib_node_desc_t *p_nd = &(node_record->node_desc);
> >  	if (p_ni->node_type == IB_NODE_TYPE_CA)
> >  	{
> > +		char desc[sizeof(p_nd->description) + 1];
> > +		memcpy(desc, p_nd->description, sizeof(p_nd->description));
> > +		desc[sizeof(desc) - 1] = '\0';
> >  		printf("%6d  \"%s\"\n",
> > -		       cl_ntoh16(node_record->lid),
> > -		       node_record->node_desc.description);
> > +		       cl_ntoh16(node_record->lid), desc);
> >  	}
> >  }
> 
> Would it not be simpler, and cleaner, to limit the string width in printf:
>   		printf("%6d  \"%.*s\"\n",
> 		       cl_ntoh16(node_record->lid),
> 			sizeof(desc),
> 			node_record->node_desc.description);

This would be simpler. However some web searching shows that not all
printf() implementation permits not null terminated arrays even when
precision is specified (some issues were reported even with glibc-2.3.2).

OTOH I understand your concerns and hate this stupid copying. Originally
wanted just to terminate node_desc.description array by '\0', but then
potentially this array can be truncated.

Sasha