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

[opticron]

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


Based on conversation here, it looks like this is waiting for an updated patch that includes only a,b,sizeof(a) and a,b,sizeof(a)-1 and excludes utils.  I'd also recommend leaving out app_rpt since it's going away soon.

- opticron


On Feb. 10, 2012, 8:29 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1732/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2012, 8:29 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 354785 
>   /trunk/addons/ooh323c/src/ooCalls.c 354785 
>   /trunk/addons/ooh323c/src/ooCmdChannel.c 354785 
>   /trunk/addons/ooh323c/src/ooh323ep.c 354785 
>   /trunk/addons/ooh323c/src/ooq931.c 354785 
>   /trunk/apps/app_rpt.c 354785 
>   /trunk/channels/iax2-provision.c 354785 
>   /trunk/pbx/dundi-parser.c 354785 
>   /trunk/res/ael/ael_lex.c 354785 
>   /trunk/res/snmp/agent.c 354785 
>   /trunk/utils/ael_main.c 354785 
>   /trunk/utils/astman.c 354785 
>   /trunk/utils/db1-ast/hash/ndbm.c 354785 
>   /trunk/utils/muted.c 354785 
> 
> 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/20120228/bcd344e0/attachment.htm>