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

[Terry Wilson]

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

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/20120211/87d6337a/attachment.htm>