[asterisk-dev] russell: trunk r327045 - in /trunk: ./channels/chan_dahdi.c

[Tony Mountifield]

In article <E1QfD9e-0003zi-9v at wibble.digium.internal>,
SVN commits to the Digium repositories <svn-commits at lists.digium.com> wrote:
> Author: russell
> Date: Fri Jul  8 10:39:42 2011
> New Revision: 327045
> 
> URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=327045
> Log:
> Merged revisions 327044 via svnmerge from 
> https://origsvn.digium.com/svn/asterisk/branches/1.8
> 
> ........
>   r327044 | russell | 2011-07-08 10:28:44 -0500 (Fri, 08 Jul 2011) | 2 lines
>   
>   Resolve some set-but-unused-variable warnings.
> ........

This looks wrong on a couple of counts, and appears to be an instance
of "never test for an error you don't know how to handle"!

Please see below (I'll just pick on one example):

> Modified:
>     trunk/   (props changed)
>     trunk/channels/chan_dahdi.c
> 
> Propchange: trunk/
> ------------------------------------------------------------------------------
> Binary property 'branch-1.8-merged' - no diff available.
> 
> Modified: trunk/channels/chan_dahdi.c
> URL:
> http://svnview.digium.com/svn/asterisk/trunk/channels/chan_dahdi.c?view=diff&rev=327045&r1=327044&r2=327045
> ==============================================================================
> --- trunk/channels/chan_dahdi.c (original)
> +++ trunk/channels/chan_dahdi.c Fri Jul  8 10:39:42 2011
> @@ -3072,9 +3072,9 @@
>  #if defined(HAVE_PRI)
>  static void my_handle_dchan_exception(struct sig_pri_span *pri, int index)
>  {
> -	int x, res;
> -
> -	res = ioctl(pri->fds[index], DAHDI_GETEVENT, &x);
> +	int x;
> +
> +	ioctl(pri->fds[index], DAHDI_GETEVENT, &x);
>  	if (x) {
>  		ast_log(LOG_NOTICE, "PRI got event: %s (%d) on D-channel of span %d\n", event2str(x), x,
> pri->span);
>  	}

Presumably if the ioctl() fails (I haven't checked whether it can fail),
x would not be updated. Since x is also uninitialised, it would be
undefined whether the log message gets printed or not.

I would be inclined to test the return value of ioctl() on principle,
and perform error exit or recovery if it is < 0.

Cheers
Tony
-- 
Tony Mountifield
Work: tony at softins.co.uk - http://www.softins.co.uk
Play: tony at mountifield.org - http://tony.mountifield.org