[Home]

Summary:ASTERISK-12491: [patch] [branch] [appdocsxml] Some fixes, and trying to start an entry point for patches to this branch
Reporter:Eliel Sardanons (eliel)Labels:
Date Opened:2008-07-30 23:50:44Date Closed:2008-07-31 12:31:32
Priority:MajorRegression?No
Status:Closed/CompleteComponents:General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) appdocxml.patch
Description:I have many questions about the ideas about the implementation, what I have done in this patch is a code review with minor fixes, and I was trying to understand the idea behind the code, Please don't close this issue if the patch is not ready, so I can continue submitting patches for this branch.
Comments:By: Eliel Sardanons (eliel) 2008-07-30 23:59:05

Fixes:
   - Memory was being overwritten when creating the path string, you didn't count the '/'.
   - It it not necessary to fclose(NULL), when fopen() failed to open 'xmldoc'.
   - ast_config_destroy() when not using anymore the allocated config structure.

Changes:
   - The Makefile should install doc/core-*.xml (and not only core-en.xml) to /var/lib/asterisk (if more languages are created their will be put there).
   - When opening the documentation file, open core-<language>.xml.
   - language was setted as en_US, es_SP, etc, so, the main filename was changed to core-en_US.xml.    
   - finish adding 'XML_DOCUMENTATION' preprocesor checks to avoid compiling code that refers to xml_documentation.
   - Minor syntax changes:
        Instead of using a looooong for(init;cond;inc) with a biiig 'init' and a biiig 'inc' use:
        init;
        while(cond) {  ; inc;}
        making code more readable.

I hope this could start a discussion to clarify some ideas...
By: Brandon Kruse (bkruse) 2008-07-31 10:25:46

Great, being committed now.

I will request you get direct access to the branch, this is good work.

Only comments: on the big for loop, I cut that out of menuselect. I do like your method much better, as it IS cleaner. Did you do any functional testing to see if the basics were still in operation?

Thanks!

-bk
By: Eliel Sardanons (eliel) 2008-07-31 10:36:35

Yes, I have tested my patch.
By: Digium Subversion (svnbot) 2008-07-31 12:31:25

Repository: asterisk
Revision: 134860

U   team/group/appdocsxml/Makefile
U   team/group/appdocsxml/main/pbx.c

------------------------------------------------------------------------
r134860 | bkruse | 2008-07-31 12:31:24 -0500 (Thu, 31 Jul 2008) | 19 lines

Fixes:
   - Memory was being overwritten when creating the path string, you didn't count the '/'.
   - It it not necessary to fclose(NULL), when fopen() failed to open 'xmldoc'.
   - ast_config_destroy() when not using anymore the allocated config structure.

Changes:
   - The Makefile should install doc/core-*.xml (and not only core-en.xml) to /var/lib/asterisk (if more languages are created their will be put there).
   - When opening the documentation file, open core-<language>.xml.
   - language was setted as en_US, es_SP, etc, so, the main filename was changed to core-en_US.xml.
   - finish adding 'XML_DOCUMENTATION' preprocesor checks to avoid compiling code that refers to xml_documentation.
   - Minor syntax changes:
        Instead of using a looooong for(init;cond;inc) with a biiig 'init' and a biiig 'inc' use:
        init;
        while(cond) { ; inc;}
        making code more readable.

(closes issue ASTERISK-12491)
(patch by elliel)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=134860