389-ds-base

#48048 Fix coverity issues - 2015/2/24, 2015/3/1
Closed: wontfix Opened by nhosoi.

Closed: wontfix
Reopen Issue

No Description Provided

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]:

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?

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:
{{{

ifdef HAVE_LONG_LONG

/ Keep this in sync with prlong.h. /

if PR_BYTES_PER_LONG == 8 && !defined(PR_ALTERNATE_INT64_TYPEDEF)

typedef long PRInt64;
typedef unsigned long PRUint64;
}}}

So we could do something like this:
{{{

if defined(HAVE_LONG_LONG) && PR_BYTES_PER_LONG == 8 && !defined(PR_ALTERNATE_INT64_TYPEDEF)

define NSPRIu64 "lu"

define NSPRI64 "l"

else / just assume long long is used /

define NSPRIu64 "llu"

define NSPRI64 "ll"

endif

}}}

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 1f59d618b7cadc7df46c05025cf2fd93f0449654
commit cd0e901f67b6ae045ba1c8a4697dd4c5506ebf19
commit 52d2f9aae4c8ad8e910dbd7dc1e2ca00ea233796
commit c1a8ba4adb798619045c83c4f9cb4a6fe0e5999d
commit 9ae878c2ff6475cabb01a035902addd8816c93fe
commit c1fa27dfefd460cf9f911c25b3c44de2663edea5
commit 3e3ed4869d6b10c0d8459d400c06603f3d0f337e
commit c703a2b994b3c9b0014e75cadcd8ed1a55ae03b7
commit f0d49d2d517186af406e07cd9e9d4c0bcff31160
commit b9723db9e5ff82712e46738fa810f728b2b6a333
commit 26f1ba532699af601cca4bee4cffcdd75c2c8fd7
commit 500e8be08158634526a92791f283c3ee210a279a
commit 13184ae13da8594b85efc7fff5317bbe2791c848
commit bbf65209fa15dce7b452ebac0d5ea66bd9466625
commit f3b463fb3aeb7bc493b361033ed2fabf56b478e2
commit deaac70c61f8c4df44635035627d550f04ea3a7d
commit acc0a2919b31b32383de90a17b7bdc5caa43939b
commit c88c7273a411b88d26370536792530df882524d7
commit 2263bd9fab124dbd84a58a2b7d4ab8e3ee84e2d4
commit b6d259dfeca81ab8f0e443b6981131d0eee38bb0
commit 0cbd5bde9afafb0e8a1ee727c5962b4e9d4cc84f
commit 4d60b688b286673ac4613c03b07e56deee3e0fa4
commit 18b44d139ad882193cd092b595dd34c6606d553f
commit 76b34e121b23956d9f29a01d3713e6f89b10a5a6
commit a7eb4e049ffa3f76e29faa7d01611ee23028d581
commit 1b0d58a5983b0b6a3b8b5ba6b7b4796782b75be1
commit 588ea1ff3aaf45bb28c2d928bd4a6b877a3a4a09
commit 7348992c874b05f4b39a49d48920563e73d82054
commit f91b5826a57304011f7a027d05a20a0ed05bf3ea
commit 6cb6ef1308e1676cf132e312a45eef3716150dff
commit bb5b5be69acf72429e824e304c5b39c16764f8d1
commit 6be713c0efe4a100cd86150b53c9edc5b0ac875d
commit c097f15b940d4192bc05f1b43cdca35697f96fd3
commit d1574aa713ead993f224609cec135f4bc8ff716e
commit 3bf2d596dec57d3ba1496bc1c45bb9d1fc1f815f
commit fc0036e2d35e189c2a34f4722fbebda548cf94b9
commit 7039bca37ebfe565ff3f3a1077d3a70e35061986
commit 94d4b10ea12e063e78c03ef64ba5c8cfb71286e1
commit a10a1de9f1b9510c12356f1608819fbe8d76fa35
commit 5ce33c406aa22ef13701dee508744bfdf23c6d4a

Pushed to 389-ds-base-1.3.3:
79a8898..27c382f 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 27c382fc9024abb524d8462ee61bf1cf6506e97c
commit cc5aba3f2628947502b7e76a52d6d1e24aaf0bf2
commit c1190b4b63eb4e360f991e59dfe49212308acbb5
commit 67024620d55223728ab50dfc8592e4a9b4eafdc2
commit 20ef9c95e8f4bd4f8adecc2d36b463cf35f3a180
commit 8c84bdf0c9e337d22fd42779e12bef38c24ccde1
commit 13da51ef8c052ac600cfeec1738d04d9926bf4ce
commit 9542aa4504cfcab89ba8ffe842f44923e06d82df
commit b22442098821af8904604fb5f0bb601d9d09cee9
commit 0b47821eec89260a1c5f6c8e39942ab9c6311240
commit 1acdd9bae6d3fad2d76aa427074e7fd9c3ad4a74
commit 29260b7878f74284312abd031e08d4682f7a6e4f
commit c080567a883cfd348e0c27b8b1f9419b581bb9ee
commit 2e7ed6ab7a5e3a674f6476e2a4efb495db94f7f9
commit 26d795e3b6a454ae01265e8a081ba9bd7bfb52b2
commit 9c33372b1c8eb7e7c876d7389b0af1c77c46653e
commit 3f7e0d45cc44bd01fc34369292819b31419346d6
commit 2b6c6869e81649a30dd9c4cfa123f34970de4d9a
commit 2828f8d9bbd457e4513a028115438b9dd7e36ae1
commit df8302286e5437e6ca186775a9980a5d62e44c9a
commit 92132539a0807af1b25f6537be36b46fa397a2e8
commit 8cf900095915b9173554158a80164eb7005fbae4
commit a50a176f2577ffdef2840bb7a30684dfd31cf24c
commit b9cd9c96ace663cc022ba3cfec18c300817e74a0
commit 4014a80da64de52462de96ee97dacf0dc9ef24b1
commit 7c408d3272f67c7cca60b1a3b90ed540c5bc2a39
commit 5f34a0041d0de02a2dce17c5b75b769e5b031f8f
commit 3e5066fbf69f23d11e361292326f818d6f914fc0
commit f8c78d8f0b94cbc2c6976f00f2944fdbf7c0075a
commit 350dd7e0120bf71918e91c5e5ebaf162c18ba670
commit 37634761be40395e27d46e73b75f62d1dcb559dc
commit 4122156a9bc5275e6dc4ab7504262dd8b61b947c
commit fcfceb286d0743e93b242e1feaae11a42938e077
commit 62c01b7e4dedad82712530bd4e89d426a0f0095d
commit 17eb3cbe8eb236289b60d8933e9ee79bcaf13f2c
commit e52e756ad8b237e91aa12a2756def173e1d7318a
commit 0dec9f13896b21c686d8c9388b7dd805494aa19d
commit 6974f19fcbd1a47a16748490e7655e74a5950a83
commit 2a571d5ae647af29db76cffbb8f17b609a49d5df
commit 97cf7278ce28c02823a671a44ef9d6e63e862fa9

One more for PRInt64; getting rid of casting to (long long int)
Master:
1f59d61..8f1eef1 master -> master
commit 8f1eef1b53a88e5d42e3313eae5fe63b9a7dd99b

389-ds-base-1.3.3:
27c382f..a875da1 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit a875da1d99f05e15e0ae7f1cd57049f35209044d

Thanks for reviewing the patch, Rich!!

Pushed to master:
3701875..f6eeaf9 master -> master
commit f6eeaf93ff608e1381063155afa9acea6c3d8045

Pushed to 389-ds-base-1.3.3:
d6f7b97..393a939 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 393a939fe8e33c4d9cedf3b3b6f7b0757f081a4f

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.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)
Metadata
None
None
None
Normal
ack
Community
None
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.