Created attachment 120262 [details] [review] patch file for misuse enum Misuse of Enum in bus_config_load() In bus_config_load, an enum-typed expression is used in a Boolean conditional context. This contribution fixes enum being treated as bool in below code statement by checking it with appropiate enum data type: if (!XML_Parse (expat, data_str, _dbus_string_get_length (&data), TRUE)) The test suite works fine after applying the proposed patch. Summary is as below: ============================================================================ Testsuite summary for dbus 1.10.99 ============================================================================ # TOTAL: 18 # PASS: 18 # SKIP: 0 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 ============================================================================
(In reply to Deepika Aggarwal from comment #0) > In bus_config_load, an enum-typed expression is used in a Boolean > conditional context. Is this invalid C? I would expect use of an enum-typed expression in boolean context to be the same as use of a plain integer in boolean context, i.e. it is true if its numeric value is nonzero. XML_STATUS_ERROR is zero. The members of XML_Status appear to be XML_STATUS_ERROR = 0, XML_STATUS_OK = 1 and XML_STATUS_SUSPENDED = 2. We never actually suspend parsing, so in practice we'll only get ERROR or OK. It seems XML_Parse() originally returned int, with values 0 or 1, and the XML_Status enum was added later: https://mail.python.org/pipermail/expat-discuss/2002-August/000602.html
Comment on attachment 120262 [details] [review] patch file for misuse enum Review of attachment 120262 [details] [review]: ----------------------------------------------------------------- I don't object to being more explicit about the comparison; the feature of returning an XML_Status instead of a plain integer seems to be old enough that it shouldn't cause compatibility issues in practice. This patch seems wrong, though. If you revise the patch please attach it in "git format-patch" format, so that it will be correctly attributed to you. With a plain diff I have to guess what name and email address (if any) you want to end up in the project's git history. ::: dbus/bus/config-loader-expat.c @@ +242,4 @@ > > data_str = _dbus_string_get_const_data (&data); > > + if (XML_Parse (expat, data_str, _dbus_string_get_length (&data), TRUE) != XML_STATUS_ERROR) I'm surprised this passes tests, because its sense is the reverse of what it should be. It used to be if (!XML_Parse (...)) which is equivalent to if (XML_Parse (...) == 0) but you've changed it into if (XML_Parse (...) != 0) Please check that you are running the full test suite (./configure --enable-tests - you will need GLib and Python, and the resulting binaries are not recommended for production use).
Created attachment 120293 [details] [review] Revised patch for misuse enum comparison fix
Created attachment 120294 [details] Revised patch full testsuite run log files Testuite run log file
Pl. find attached the correct updated and formatted patch. (0001-Misuse-of-Enum-in-bus_config_load-during-comparison.patch) The previous patch got uploaded by mistake. Its better to check specifically with enum status XML_STATUS_ERROR though the enum value is 0 only. It will avoid any chance of mistake in future and better readability as the enum has other non zero values also. I ran the full test suite with this patch (including Glib and Python) Below are the test results: ============================================================================ Testsuite summary for dbus 1.10.99 ============================================================================ # TOTAL: 99 # PASS: 99 # SKIP: 0 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 ============================================================================ Also, attached are the full test suite run log file. (dbustestsuite_misuseEnum.log)
Fixed in git for 1.11.4, thanks. This doesn't seem to be a user-observable bug, so I'm not backporting it to the 1.10.x stable branch.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.