| Summary: | ASTERISK-10357: [patch] ast_append_ha cleanup | ||
| Reporter: | Dmitry Andrianov (dimas) | Labels: | |
| Date Opened: | 2007-09-21 02:32:38 | Date Closed: | 2007-10-18 02:02:54 |
| Priority: | Trivial | Regression? | No |
| Status: | Closed/Complete | Components: | Core/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| 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. ****** ADDITIONAL INFORMATION ****** What I did exacly: 1. Report error when can't allocate memory 2. if no mask specified, just use 0xFFFFFFFF instead of __parsing__ 255.255.255.255 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 ------------------------------------------------------------------------ | ||