attachment 0002-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0003-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0004-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0005-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0006-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0007-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0008-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0009-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0010-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0011-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0012-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0013-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0014-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0015-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0016-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0017-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0018-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0019-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0020-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0021-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0022-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0023-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0024-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0025-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0026-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0027-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0028-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0029-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0030-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0031-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0032-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0033-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0034-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0035-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0036-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0037-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0038-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0039-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
attachment 0040-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
in https://fedorahosted.org/389/attachment/ticket/48048/0006-Ticket-48048-Fix-coverity-issues-2015-2-24.patch and https://fedorahosted.org/389/attachment/ticket/48048/0032-Ticket-48048-Fix-coverity-issues-2015-2-24.patch - can you just use {{{ PRUint64 o_connid = 0xffffffffffffffff; / no op / }}} and avoid having to cast anything?
in https://fedorahosted.org/389/attachment/ticket/48048/0018-Ticket-48048-Fix-coverity-issues-2015-2-24.patch Is rc initialized or otherwise set?
Replying to [comment:3 rmeggins]:
Yeah, I'd also like to avoid casting. The original code is the old line 305. o_connid has type PRUint64, which is casted to (long long unsigned int) for "conn=%" NSPRIu64. Those casts are now removed as in the new line 310.... {{{ 302 307 slapi_log_error (loglevel, plugin_name, 303 308 "conn=%" NSPRIu64 " op=%d (main): Deny %s on entry(%s)" 304 ": readonly backend\n", 305 (long long unsigned int)op->o_connid, op->o_opid, 309 ": readonly backend\n", 310 o_connid, o_opid, }}}
in https://fedorahosted.org/389/attachment/ticket/48048/0018-Ticket-48048-Fix-coverity-issues-2015-2-24.patch Is rc initialized or otherwise set? rc is initialized with -1. 1185 int rc = -1;
Right. You got rid of all of the casting except this one: {{{
265 if (op) { 266 o_connid = (long long unsigned int)op->o_connid; 267 o_opid = op->o_opid; 268 }
}}} Can you declare PRUint64 o_connid instead of long long unsigned int o_connid? Then you shouldn't need the cast here because {{{ typedef struct op { ... PRUint64 o_connid; / id of conn initiating this op; for logging only / }}}
Replying to [comment:5 rmeggins]:
Right. You got rid of all of the casting except this one:
Ah, I see. You want to avoid all of them... :) Let me try the change...
Replying to [comment:6 nhosoi]:
Replying to [comment:5 rmeggins]: Right. You got rid of all of the casting except this one: Ah, I see. You want to avoid all of them... :) Let me try the change...
Well, now, we have to face the original compiler warning problem. (we put lots of cast in slapi_log_error to work around this...)
ldap/servers/plugins/acl/acl.c:312:5: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 4 has type 'PRUint64' [-Wformat=]
slapi-plugin.h:#define NSPRIu64 "llu"
260 PRUint64 o_connid = 0xffffffffffffffff; / no op / ... 266 o_connid = op->o_connid; ... 307 slapi_log_error (loglevel, plugin_name, 308 "conn=%" NSPRIu64 " op=%d (main): Deny %s on entry(%s)" 309 ": readonly backend\n", 310 o_connid, o_opid, 311 acl_access2str(access), 312 n_edn);
Would you mind introducing this "#ifdef CPU_x86_64"? This cleans up the above warning. {{{ diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index bc68157..c559852 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -89,8 +89,13 @@ NSPR_API(PRUint32) PR_fprintf(struct PRFileDesc fd, const char fmt, ...) / NSPR uses the print macros a bit differently than ANSI C. We * need to use ll for a 64-bit integer, even when a long is 64-bit. / +#ifdef CPU_x86_64 +#define NSPRIu64 "lu" +#define NSPRI64 "l" +#else #define NSPRIu64 "llu" #define NSPRI64 "ll" +#endif }}}
Ok, I see what we want - If PRUint64 is defined as unsigned log, use %lu, otherwise, use %llu. We should use the same macro that prtypes.h uses: {{{
/ Keep this in sync with prlong.h. /
typedef long PRInt64; typedef unsigned long PRUint64; }}}
So we could do something like this: {{{
}}}
Yep, it works nicely. I'm putting the change to one of the patches (https://fedorahosted.org/389/attachment/ticket/48048/0006-Ticket-48048-Fix-coverity-issues-2015-2-24.patch) and modifying the rest by getting rid of the casts (I think there are more outside of the patches I proposed...)
Thanks a lot, Rich!!
git patch file (master) -- get rid of (long long unsigned int) cast 0006-Ticket-48048-Fix-coverity-issues-2015-2-24.2.patch
git patch file (master) -- get rid of (long long unsigned int) cast 0032-Ticket-48048-Fix-coverity-issues-2015-2-24.2.patch
git patch file (master) -- get rid of (long long unsigned int) cast 0041-Ticket-48048-Fix-coverity-issues-2015-2-24.patch
Reviewed and advised by Rich (Thank you!!)
Pushed to master: 1aeaf34..1f59d61 master -> master commit 1f59d61 commit cd0e901 commit 52d2f9a commit c1a8ba4 commit 9ae878c commit c1fa27d commit 3e3ed48 commit c703a2b commit f0d49d2 commit b9723db commit 26f1ba5 commit 500e8be commit 13184ae commit bbf6520 commit f3b463f commit deaac70 commit acc0a29 commit c88c727 commit 2263bd9 commit b6d259d commit 0cbd5bd commit 4d60b68 commit 18b44d1 commit 76b34e1 commit a7eb4e0 commit 1b0d58a commit 588ea1f commit 7348992 commit f91b582 commit 6cb6ef1 commit bb5b5be commit 6be713c commit c097f15 commit d1574aa commit 3bf2d59 commit fc0036e commit 7039bca commit 94d4b10 commit a10a1de commit 5ce33c4
Pushed to 389-ds-base-1.3.3: 79a8898..27c382f 389-ds-base-1.3.3 -> 389-ds-base-1.3.3 commit 27c382f commit cc5aba3 commit c1190b4 commit 6702462 commit 20ef9c9 commit 8c84bdf commit 13da51e commit 9542aa4 commit b224420 commit 0b47821 commit 1acdd9b commit 29260b7 commit c080567 commit 2e7ed6a commit 26d795e commit 9c33372 commit 3f7e0d4 commit 2b6c686 commit 2828f8d commit df83022 commit 9213253 commit 8cf9000 commit a50a176 commit b9cd9c9 commit 4014a80 commit 7c408d3 commit 5f34a00 commit 3e5066f commit f8c78d8 commit 350dd7e commit 3763476 commit 4122156 commit fcfceb2 commit 62c01b7 commit 17eb3cb commit e52e756 commit 0dec9f1 commit 6974f19 commit 2a571d5 commit 97cf727
One more for PRInt64; getting rid of casting to (long long int) Master: 1f59d61..8f1eef1 master -> master commit 8f1eef1
389-ds-base-1.3.3: 27c382f..a875da1 389-ds-base-1.3.3 -> 389-ds-base-1.3.3 commit a875da1
git patch file (master) 0001-Ticket-48048-Fix-coverity-issues-2015-3-1.patch
Thanks for reviewing the patch, Rich!!
Pushed to master: 3701875..f6eeaf9 master -> master commit f6eeaf9
Pushed to 389-ds-base-1.3.3: d6f7b97..393a939 389-ds-base-1.3.3 -> 389-ds-base-1.3.3 commit 393a939
Metadata Update from @nhosoi: - Issue assigned to nhosoi - Issue set to the milestone: 1.3.3.9
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/1379
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.