[asterisk-dev] [Code Review]: Convert two classes of strncpy usage to ast_copy_string

[Terry Wilson]

> On March 8, 2012, 8:34 p.m., Tilghman Lesher wrote:
> > /trunk/apps/app_rpt.c, line 1428
> > <https://reviewboard.asterisk.org/r/1732/diff/3/?file=25943#file25943line1428>
> >
> >     Um, isn't this whole file about to be deleted from trunk?

Yep. And if they delete it before this gets a ship it, they won't have to delete the extra few characters added by this patch. :-)


- Terry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1732/#review5785
-----------------------------------------------------------


On March 8, 2012, 6:22 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1732/
> -----------------------------------------------------------
> 
> (Updated March 8, 2012, 6:22 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> One of the janitor projects is to replace strncpy with ast_copy_string where possible. As a brief example of using the coccinelle tool, I thought I'd post an example for using it. There are some cases of strncpy which where ast_copy_string wouldn't be appropriate--sepcifically when it is used to copy non-NULL terminated buffers (instead of memcpy, which should be used for that). Two cases that I think should be completely safe are where we have something like strncpy(src, dst, ${something} - 1) and strncpy(src, dst, sizeof(src)) (which is a bug waiting to happen). 
> 
> Here is the coccinelle patch to do the translation:
> 
> strncpy.cocci
> @@
> expression a, b, c;
> @@
> 
> - strncpy(a, b, c - 1)
> + ast_copy_string(a, b, c)
> 
> @@
> expression a, b;
> @@
> 
> - strncpy(a, b, sizeof(a))
> + ast_copy_string(a, b, sizeof(a))
> 
> Coccinelle needs a little help with some of the macros in Asterisk--so far it looks like the ones in linkedlists.h and app.h give it the most trouble. You can generate a macro file to use by doing:
> 
> spatch -extract_macros include/asterisk/linkedlists.h > /tmp/spatch.h
> spatch -extract_macros include/asterisk/app.h >> /tmp/spatch.h
> 
> I've found that doing all of the includes sometimes makes it miss stuff for some reason...
> 
> Then to run the patch file on all the files in the asterisk directory:
> spatch -sp_file strncpy.cocci -preprocess -c -in_place -I include/ -local_includes -macro_file /tmp/spatch.h -smpl_spacing -dir .
> 
> You may have to fix some stuff up manually, but it goes a long way to making things a lot easier. This is a simple example of how the channel opaquification is done. I'll write this all up on the Wiki, but am basically just doing a quick brain dump before I leave for the night.
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/chan_ooh323.c 358725 
>   /trunk/apps/app_alarmreceiver.c 358725 
>   /trunk/apps/app_rpt.c 358725 
>   /trunk/channels/iax2-provision.c 358725 
>   /trunk/res/snmp/agent.c 358725 
> 
> Diff: https://reviewboard.asterisk.org/r/1732/diff
> 
> 
> Testing
> -------
> 
> It compiles.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120309/b4c192f2/attachment.htm>