1 Rules for commits on the xmlsec module
2 =========================================
4 0) DO NOT COMMIT DIRECTLY !
5 If you have a patch send a mail to xmlsec@aleksey.com mailing
6 list (you must be subscribed to the list, go to
7 http://www.aleksey.com/mailman/listinfo/xmlsec to subscribe).
9 If there is a problem in xmlsec module that prevents you
10 from building other major components then feel free to patch
11 first and then send a mail. This is an EXCEPTIONAL case and
12 you should be VERY carefull when you are doing this.
14 Igor Zlatkovic get an exception for the send before commit rule.
17 - Formatting. Just for clarification, the formating is:
19 tab size=8;indentation=4;insert spaces=yes
21 - Use explicit "!= NULL", "!= 0", etc. This makes code
22 easier to read and remove warnings on some platform.
31 - Put figure brackets '{}' even if you have only one operator
32 in "if", "for", etc. This also makes code easier to read and
33 saves a lot of time when you need to quickly change something.
43 - Use round brackets '()' in conditions to show the precedence order.
44 I don't remember what goes first '<<' or '*', do you?
47 if(privkey == NULL || pubkey == NULL)
49 if((privkey == NULL) || (pubkey == NULL))
51 - Use round brackets '()' for "return".
58 - Check for warnings! Use "--enable-pedantic" option
59 for "configure.in" script to enable as much warnings as possible.
60 Your patch should produce no new warnings and if you'll
61 see something that you can fix, then do it.
63 - Check for memory leaks. There is a built in support for
64 valgrind (http://devel-home.kde.org/~sewardj/). In order to use it,
65 use "enable_static_linking" option for "configure.in" script to
66 force static linking of xmlsec command line utility and run
67 "make memcheck" from the top xmlsec source folder. The results are printed
68 at the end. More detailed logs could be found in /tmp/test*.log files.
71 - You should trust nobody! Anyone can fool you: user or another application
72 might provide you incorrect data; call to xmlsec or system function might
73 fail with an error code; worse, the same call might fail but the return
74 code is "success" and so on. The patch fixes a lot of places where the
75 original code failed to check input data or function return values.
76 One of my favorite examples is the code that *silently* assumed that
77 base64 decoded value of a RSA public exponent obtained from XML fits
78 in a DWORD. And after that the code did memcpy to copy from xmlSecBuffer
79 to a DWORD variable *without* checking how much data are actualy copied!
80 The trivial DoS attack (at least DoS!!!) is to put very long base64 string
81 in XML file and enjoy the server crash.
82 One of the strongest sides of xmlsec library is that there are very few
83 known ways to crash it (and all of them are related to running the
84 application in an environment with a very limited memory to force a malloc
85 failure). To be a little paranoid is good in this context :)
87 - malloc/free vs. xmlMalloc/xmlFree
88 xmlsec library use libxml2 memory management functions. This provides an
89 easy way to replace default memory management functions with custom ones.
90 And this might be very usefull in some cases.
91 Note that crypto library might use a different memory management
92 functions! Be very carefully to do not mix them (i.e. get memory
93 allocated by crypto library function and free it with xmFree).
95 - Errors reporting (XMLSEC_ERRORS_R_XMLSEC_FAILED vs. XMLSEC_ERRORS_R_CRYPTO_FAILED)
96 The correct usage rule is:
97 if the failed function starts with "xmlSec" then use
98 XMLSEC_ERRORS_R_XMLSEC_FAILED
99 else if it is xmlMalloc/xmlFree/xmlStrdup/etc then use
100 XMLSEC_ERRORS_R_MALLOC_FAILED
101 else if the function starts with "xml" or "xslt" (i.e. it comes
102 from libxml or libxslt) then use
103 XMLSEC_ERRORS_R_XML_FAILED
104 else if it is related to IO (fopen, fread, fwrite, etc.) then use
105 XMLSEC_ERRORS_R_IO_FAILED
106 else if the function could be used only from xmlsec-crypto (i.e.
107 it is crypto engine related) then use
108 XMLSEC_ERRORS_R_CRYPTO_FAILED
109 else if there is another reason (invalid data, invalid size, etc.)
110 corresponding error reason should be used
112 it is something new and should be discussed
114 Correct error reason is very important. For example, some applications
115 ignore all the XMLSEC_ERRORS_R_XMLSEC_FAILED errors to get to the bottom of
116 the errors stack and report the actual problem.
118 - Errors reporting: "size=%d;error=%d" instead of "size %d, error: %d":
119 It would be great if xmlsec-crypto libraries can follow the error message
120 standard adopted in the other files of xmlsec library:
121 "<name1>=<value1>;<name2>=<value2>;..."
122 This greatly helps when one needs to write a logs parser. For example, to
123 find the reason of memory allocation failures.
125 3) Preparing and submiting a patch.
126 If you want to submit a patch please do following:
127 - Get a CVS source copy (see http://www.aleksey.com/xmlsec/download.html).
128 It's much easier to prepare patch from CVS than to diff two set of files.
129 - Test your patch! Make sure that your patch complain with xmlsec coding
130 style (see above) and that you don't introduce new warnings or memory leaks
131 (also see above). If you have a new functionality in the patch,
132 do not forget to add a test case(s) in the xmlsec test suite.
133 - If you have new files in your patch mark them "to be added" with
135 command. If you have binary files, do not forget to use '-kb' option
136 cvs add -kb <filename>
137 If you have new folders in your patch and you don't have write access to CVS,
138 send a mail to xmlsec@aleksey.com and I'll create them for you.
139 - Prepare patch by running diff command from the top of the source tree:
140 cvs -z3 diff -uN [<file or folder names>...] > <output filename>
141 The file or folder names are optional and you can use it to save
142 yourself some time. "-u" option produces a human readble diff,
143 "-N" option includes to the diff new files created on prevous step.
144 Finally, "-z3" forces cvs to compress the network traffic and make things
145 faster. Please use ".diff" extension in your output filename. This will
146 add colors to my editor when I would be looking at it :)
147 - Gzip or zip your diff file! Don't send plain diff file because some mailers
149 - Send your patch along with a short description of the problem or feature
150 you are fixing/implementing to the xmlsec@aleksey.com mailing list
151 (you must be subscribed to the list, go to http://www.aleksey.com/mailman/listinfo/xmlsec to subscribe).
152 If you are fixing a bug, it might be a good idea to bugzilla it first
153 (http://www.aleksey.com/xmlsec/bugs.html) for the record. Do not forget
154 to put link or bug number in your message if the bug is in bugzilla.