Re: [PATCH] gpu: drm: drm_dp_mst_topology.c: Fix improper use of strncat.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Oct 05, 2014 at 11:46:03PM +0200, Rickard Strandqvist wrote:
> I have now eliminate the need to use the temporary string,
> and therefore also the use of strncat.
> And I think this code is clearer and more effective.
> 
The code is definitely less clear.

> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c |   15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index ac3c273..c3f6571 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -997,17 +997,18 @@ static void build_mst_prop_path(struct drm_dp_mst_port *port,
>  				struct drm_dp_mst_branch *mstb,
>  				char *proppath)
>  {
> -	int i;
> -	char temp[8];
> -	snprintf(proppath, 255, "mst:%d", mstb->mgr->conn_base_id);
> +	int i, len;
> +	static const int proppath_len = 255;

Length should be passed by the caller.

> +	memset(proppath, 0, proppath_len);

Why?
> +	len = snprintf(proppath, proppath_len, "mst:%d", mstb->mgr->conn_base_id);
>  	for (i = 0; i < (mstb->lct - 1); i++) {
>  		int shift = (i % 2) ? 0 : 4;
>  		int port_num = mstb->rad[i / 2] >> shift;
> -		snprintf(temp, 8, "-%d", port_num);
> -		strncat(proppath, temp, 255);
> +		len += snprintf(&proppath[(len < proppath_len ? len : 0)],
> +						proppath_len - len, "-%d", port_num);

And if the space ran out you just overwrite from the start of the buffer
and provide negative length.

Since this function returns void (and so does its caller) it is quite
unclear what to do if the space ran out. I would just WARN.

>  	}
> -	snprintf(temp, 8, "-%d", port->port_num);
> -	strncat(proppath, temp, 255);
> +	snprintf(&proppath[(len < proppath_len ? len : 0)], proppath_len - len,
> +					"-%d", port->port_num);
>  }
>  
>  static void drm_dp_add_port(struct drm_dp_mst_branch *mstb,

-- 
Mateusz Guzik
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux