Fix compilation failure due to -Werror flag #4777
Conversation
Attempt to fix error message:
imap/http_dav.c: In function ‘parse_xml_body’:
imap/http_dav.c:3819:33: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
3819 | xmlErrorPtr err = xmlCtxtGetLastError(txn->conn->xml);
|
Signed-off-by: Athaariq Ardhiansyah <[email protected]>
Attempt to fix error message: /mnt/img/athaariq/cyrus-imapd/docsrc/imap/concepts/deployment/storage.rst:973: WARNING: duplicate term description of MTBF, other instance in glossary /mnt/img/athaariq/cyrus-imapd/docsrc/imap/concepts/deployment/storage.rst:977: WARNING: duplicate term description of HBA, other instance in glossary /mnt/img/athaariq/cyrus-imapd/docsrc/imap/concepts/features/server-aggregation.rst:392: WARNING: duplicate term description of frontend, other instance in glossary /mnt/img/athaariq/cyrus-imapd/docsrc/imap/concepts/features/server-aggregation.rst:397: WARNING: duplicate term description of backend, other instance in glossary Signed-off-by: Athaariq Ardhiansyah <[email protected]>
These steps shouldn't be necessary, the top level |
There was a problem hiding this comment.
Thanks. I appreciate the enthusiasm for cleaning up the documentation warnings, but most of your proposed changes make things worse. I've addressed each with individual comments.
The documentation warnings are not what's causing your build to fail. -Werror only applies to the C compiler, not to sphinx-build.
It looks like your build fails because of the const warning from http_dav.c. I've asked Ken to have a closer look at this one, because if the object returned by the libxml call is const (or is const in newer versions of libxml), the way we use it might be unsafe.
| xmlErrorPtr err = xmlCtxtGetLastError(txn->conn->xml); | ||
| const xmlError *err = xmlCtxtGetLastError(txn->conn->xml); |
There was a problem hiding this comment.
@ksmurchison can you have a look at this one please, I'm not familiar with the XML library.
In particular, a few lines later we save err->message in txn->error.desc. That might not be safe if we don't have lifetime ownership of the err object, which the constness might imply.
Since we aren't getting this warning/error outselves, perhaps this is an API change in the XML library that we haven't stumbled upon yet.
There was a problem hiding this comment.
Adding const seems correct as that is what the API specifies. Since we're getting the error from the XML parser context, and there is a xmlCtxtResetLastError() function, I assume its safe to reference the error until we free the parser context.
There was a problem hiding this comment.
As an information, the Arch distribution uses libxml2 v2.12.3. I see the changes at libxml2 GitLab repository which related to this issue. This is what maintainer's say:
This is a slight break of the API, but users really shouldn't modify the global error struct. The goal is to make xmlLastError use static buffers for its strings eventually. This should warn people if they're abusing the struct.
There was a problem hiding this comment.
Adding const seems correct as that is what the API specifies. Since we're getting the error from the XML parser context, and there is a xmlCtxtResetLastError() function, I assume its safe to reference the error until we free the parser context.
Alright, thanks for the clarification. Is there anything I can do?
|
Drop commit requests are fulfilled, please take a look
I tried, but it yells "No build target" error. There is no explanation or verbose message about the problem unfortunately, so I have to do that manually.
The commit 6a9d617 went first, but |
|
can this get a review/merge? |
This is an attempt to get this commands working for building the cyrus-imapd:
Without error caused by warning and
-Werrorflag. Also notes that currently I'm working with upstream's latest stable version of toolchains and dependencies. Thus, the fix might not quite relevant on Debian but definitely needed in rolling release distributions like Arch.