Summary:ASTERISK-10357: [patch] ast_append_ha cleanup
Reporter:Dmitry Andrianov (dimas)Labels:
Date Opened:2007-09-21 02:32:38Date Closed:2007-10-18 02:02:54
Versions:Frequency of
Environment:Attachments:( 0) acl.patch
( 1) acl-trunk.patch
Description:My patch just cleans up ast_append_ha code and provides better error reporting. It should not change any behaviour.


What I did exacly:
1. Report error when can't allocate memory
2. if no mask specified, just use 0xFFFFFFFF instead of __parsing__
3. For CIDR notation use single shift operation to build mask instead of looping :)
4. Log wrong CIDRs and ignore these.
5. when reporting any problem, log whole ip/mask pair not just wrong piece.
Comments:By: Russell Bryant (russell) 2007-09-21 08:59:01

ast_malloc() will generate an ast_log() message internally if the allocation fails.  Other than that, I think it looks good ...

By: Dmitry Andrianov (dimas) 2007-09-21 09:01:31

should I submit another patch or person who will be applying the patch will remove unneeded ast_log?

By: Russell Bryant (russell) 2007-09-21 09:04:54

The person applying it can do it.  I was just pointing it out just so you know for the future.

By: Russell Bryant (russell) 2007-09-21 09:05:54

And actually, since this is "code cleanup" and not a bug fix, it is only a candidate for trunk, and not 1.4.  It looks like the code in trunk is different so the patch doesn't apply.

By: Dmitry Andrianov (dimas) 2007-09-21 10:42:02

I never said it is a bugfix :)

anyway, patch for trunk provided. I also removed the unneeded warning you pointed and changed from fixed size tmp buffer to ast_strdupa when parsing the string. Not sure if you consider the last one good practice but I saw many uses of ast_strdupa in the code.

Btw, honestly, I'm not using trunk - I tested my changes for 1.4 but I can not test them for trunk.

PS: that is pity you can not accept even simple feature changes to the stable branch. For example ast_debug is just an addition of small feature - it does not break old way of using ast_log so it could be accepted for 1.4 branch without pain. Since all the code in the trunk was changed to use ast_debug while 1.4 branch still uses ast_log(LOG_DEBUG), most of patches created for 1.4 branch won't apply for trunk.

By: Digium Subversion (svnbot) 2007-10-18 02:02:54

Repository: asterisk
Revision: 86278

U   trunk/main/acl.c

r86278 | tilghman | 2007-10-18 02:02:53 -0500 (Thu, 18 Oct 2007) | 4 lines

Code cleanup of acl.c
Reported by dimas
Closes issue ASTERISK-10357