Nut upsdev - Jun 2011 - [nut-commits] svn commit r3048 - branches/nut-scanner/tools/nut-scanner

Citeren Frederic BOHE <fbohe-guest at alioth.debian.org>:
> Modified: branches/nut-scanner/tools/nut-scanner/scan_snmp.c
>
=============================================================================>
--- branches/nut-scanner/tools/nut-scanner/scan_snmp.c	Fri Jun 17
> 08:12:00 2011	(r3047)
> +++ branches/nut-scanner/tools/nut-scanner/scan_snmp.c	Tue Jun 21  
> 07:56:55 2011	(r3048)
> @@ -101,8 +101,12 @@
>  		dev->type = TYPE_SNMP;
>  		dev->driver = strdup("snmp-ups");
>  		dev->port = strdup(session->peername);
> -		snprintf(buf,sizeof(buf),"\"%s\"",
> -				 response->variables->val.string);
> +		/* get the name aof the device and add "" */
> +		memset(buf,0,sizeof(buf));
> +		buf[0] = '"';
> +		memcpy(buf+1,response->variables->val.string,
> +			response->variables->val_len);
> +		buf[1+response->variables->val_len] = '"';
>  		add_option_to_device(dev,"desc",buf);
>  		add_option_to_device(dev,"mibs",snmp_device_table[index].mib);
>  		/* SNMP v3 */
What is the reason to switch from the (buffer overflow) safe  
'snprintf' method to the above unsafe way of doing this? I'm not too
thrilled about this. If you want to copy  
'response->variables->val_len' bytes, the proper way to do this is
to
use

     snprintf(buf,sizeof(buf),"\"%.*s\"",  
(int)response->variables->val_len, response->variables->val.string);

instead.

Please remember to use the log comments to explain why a patch is  
submitted. We need these comments to generate the ChangeLog. Not doing  
this, requires someone to guess your reasons and add them manually,  
which is just a waste of effort. Thank you.

Best regards, Arjen
-- 
Please keep list traffic on the list (off-list replies will be rejected)

On Tue, 2011-06-21 at 20:12 +0200, Arjen de Korte wrote:
> Citeren Frederic BOHE <fbohe-guest at alioth.debian.org>:
> 
> > Modified: branches/nut-scanner/tools/nut-scanner/scan_snmp.c
> >
=============================================================================>
> --- branches/nut-scanner/tools/nut-scanner/scan_snmp.c	Fri Jun 17
> > 08:12:00 2011	(r3047)
> > +++ branches/nut-scanner/tools/nut-scanner/scan_snmp.c	Tue Jun 21  
> > 07:56:55 2011	(r3048)
> > @@ -101,8 +101,12 @@
> >  		dev->type = TYPE_SNMP;
> >  		dev->driver = strdup("snmp-ups");
> >  		dev->port = strdup(session->peername);
> > -		snprintf(buf,sizeof(buf),"\"%s\"",
> > -				 response->variables->val.string);
> > +		/* get the name aof the device and add "" */
> > +		memset(buf,0,sizeof(buf));
> > +		buf[0] = '"';
> > +		memcpy(buf+1,response->variables->val.string,
> > +			response->variables->val_len);
> > +		buf[1+response->variables->val_len] = '"';
> >  		add_option_to_device(dev,"desc",buf);
> >  	
add_option_to_device(dev,"mibs",snmp_device_table[index].mib);
> >  		/* SNMP v3 */
> 
> What is the reason to switch from the (buffer overflow) safe  
> 'snprintf' method to the above unsafe way of doing this? I'm
not too
> thrilled about this. If you want to copy  
> 'response->variables->val_len' bytes, the proper way to do
this is to
> use
> 
>      snprintf(buf,sizeof(buf),"\"%.*s\"",  
> (int)response->variables->val_len,
response->variables->val.string);
> 
> instead.
I just did not know this way of using printf. I've committed the change,
thanks for the tip.
> 
> Please remember to use the log comments to explain why a patch is  
> submitted. We need these comments to generate the ChangeLog. Not doing  
> this, requires someone to guess your reasons and add them manually,  
> which is just a waste of effort. Thank you.
I'm usually paying attention to my commit log. Commit 3048 was the
result of a bad manipulation of my svn interface (click OK instead of
Cancel in a commit box). 

Regards,
Fred
> 
> Best regards, Arjen
-- 
Team Open Source Eaton - http://powerquality.eaton.com

--------------------------------------------------------------------------