I'll provide here patches who perform cosmetic correctures in the SAMBA 4 code.
Created attachment 3534 [details] KERBEROS cosmetic patch
Created attachment 3535 [details] DSDB cosmetic patch
Created attachment 3536 [details] LDB library cosmetic patch
Created attachment 3537 [details] LDB backends cosmetic patch
Created attachment 3538 [details] DSDB cosmetic patch Corrected version
Created attachment 3539 [details] LDB library cosmetic patch Corrected version
Created attachment 3540 [details] LDB backends cosmetic patch Corrected version
Created attachment 3541 [details] DSDB cosmetic patch Updated DSDB cosmetic patch for changed kludge_acl module.
(In reply to comment #6) > Created an attachment (id=3539) [edit] > LDB library cosmetic patch > > Corrected version > This patch is not completely ok. While in most cases your change from a numberic code to LDB_ERR_OPERATIONS_ERROR is ok, in some other it is not. Look for example at: ldb_should_b64_encode() This function should probably really return a boolean true|false, but changing the return of 1 (which means true here) to LDB_ERR_OPERATIONS_ERROR is semantically wrong. Please check each caller and make sure that what they expect in return is actually an ldb error. I now of other places within ldb where returning 1,0,-1 has meaning and it would break if something else is returned. Simo.
1.) Add also Jelmer to this bug, because he is also interested in it. 2.) Regarding comment #9 (Simo): It's for me a bit difficult to know where the constants are appropriate and where not. Also due to the fact that I haven't written the code. It would be a lot easier to correct my patches if I knew at least the filenames (modules) or function forms (e.g. comparison functions not) where the patching isn't allowed/required.
Comment on attachment 3534 [details] KERBEROS cosmetic patch This isn't the latest version of the patch (see the GIT branch for a newer version).
(In reply to comment #10) > It's for me a bit difficult to know where the constants are appropriate and > where not. Also due to the fact that I haven't written the code. It would be a > lot easier to correct my patches if I knew at least the filenames (modules) or > function forms (e.g. comparison functions not) where the patching isn't > allowed/required. The easiest way is to use something like ctags/cscope to jump to all users of a function you modify and verify how they use the return. I did this for a sample of functions and that's how I saw the patch was not completely correct. Once you know how a calss of functions operate wrt return value you can skip strict checking for all functions of that class.
Good, in the meantime the patches are merged, but the LDB results remain to fix. I hope I'll do some investigations on how to fix it in the next days.
Created attachment 3628 [details] Last correction All patches should be applied. Here I provide a last update to satisfy Simo's concerns about "ldb_should_b64_encode()". I checked also the other functions with the "cscope" utility and found nothing special. After this is applied, it should be safe to close the bug.
Good, I've rebased my cosmetic branch now, did some checks with "cscope" and wait for the review.
My branch has been updated due to the LDB ASYNC changes.
Patches applied.