URL: https://github.com/SSSD/sssd/pull/5750 Author: pbrezina Title: #5750: fix compilation warnings Action: opened
PR body: """ None """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5750/head:pr5750 git checkout pr5750
URL: https://github.com/SSSD/sssd/pull/5750 Author: pbrezina Title: #5750: fix compilation warnings Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5750/head:pr5750 git checkout pr5750
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
alexey-tikhonov commented: """ About 2nd patch... Can't we just change signature of `sss_getenv()` to `... const char **_value`? (CC @elkoniu) """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-900195862
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
elkoniu commented: """
About 2nd patch... Can't we just change signature of `sss_getenv()` to `... const char **_value`? (CC @elkoniu)
I think the problem has philosophical nature. We are allocating data and modifying pointer inside function so in fact it is not `const` even if we consider output data pointed by it as `const`. Another issue is deallocation. I think when talloc will be releasing the data with `free()` it will complain about const pointer. """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-900202615
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
alexey-tikhonov commented: """
About 2nd patch... Can't we just change signature of `sss_getenv()` to `... const char **_value`? (CC @elkoniu)
I think the problem has philosophical nature. We are allocating data and modifying pointer inside function so in fact it is not `const` even if we consider output data pointed by it as `const`. Another issue is deallocation. I think when talloc will be releasing the data with `free()` it will complain about const pointer.
No, I don't propose a const pointer to a char (`char * const`). I propose a pointer to const char (`const char *`).
"""
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-900213809
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
elkoniu commented: """ I think @alexey-tikhonov is right, you can just change signature of `sss_getenv()` to return `const char*`. As this memory is released by talloc there will be no issue with calling `free()` on const pointer. """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-900330779
URL: https://github.com/SSSD/sssd/pull/5750 Author: pbrezina Title: #5750: fix compilation warnings Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5750/head:pr5750 git checkout pr5750
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
pbrezina commented: """ See update patch. """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-903886014
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
Label: +Waiting for review
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
alexey-tikhonov commented: """ About 1st patch: frankly, I think use of `_DEPRECATED_` keyword in libtalloc header is unjustified. But since the patch just implements/copies (simplified) original libtalloc code - ACK. """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-904501430
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
alexey-tikhonov commented: """ About 2nd patch: can we increase log level in `DEBUG(SSSDBG_TRACE_ALL, "sss_getenv() failed\n");` in `sysdb_ldb_connect()`? Otherwise patch looks good to me. """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-904522306
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
alexey-tikhonov commented: """ About 2nd patch: can we increase log level in `DEBUG(SSSDBG_TRACE_ALL, "sss_getenv() failed\n");` in `sysdb_ldb_connect()`? Otherwise patch looks good to me.
(I know this is original code, but so close)
Covscan: https://cov01.lab.eng.brq.redhat.com/covscanhub/task/230741/log/fixed.err """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-904522306
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
alexey-tikhonov commented: """ About 2nd patch: can we increase log level in `DEBUG(SSSDBG_TRACE_ALL, "sss_getenv() failed\n");` in `sysdb_ldb_connect()`? Users of this function don't quote fail reason: it's just `DEBUG(SSSDBG_CRIT_FAILURE, "sysdb_ldb_connect failed.\n");` (I know this is original code, but so close)
Otherwise patch looks good to me.
Covscan: https://cov01.lab.eng.brq.redhat.com/covscanhub/task/230741/log/fixed.err """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-904522306
URL: https://github.com/SSSD/sssd/pull/5750 Author: pbrezina Title: #5750: fix compilation warnings Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5750/head:pr5750 git checkout pr5750
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
pbrezina commented: """ See new patch. Diff: https://github.com/SSSD/sssd/compare/1944174d47711a26b2a8c3e6024b4fc056d5db0... """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-904583002
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
alexey-tikhonov commented: """ Thank you, ACK. """
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-904585632
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
Label: -Waiting for review
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
Label: +Ready to push
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
pbrezina commented: """ Pushed PR: https://github.com/SSSD/sssd/pull/5750
* `master` * 575e1899edbf3418c79aecb5e4feda868c119d53 - fix warnings around sss_getenv() * a1f7035b39d3cdc7980c2c9478f8edb12828c4b6 - remove deprecated talloc_autofree_context()
"""
See the full comment at https://github.com/SSSD/sssd/pull/5750#issuecomment-905346060
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
Label: -Accepted
URL: https://github.com/SSSD/sssd/pull/5750 Title: #5750: fix compilation warnings
Label: -Ready to push
URL: https://github.com/SSSD/sssd/pull/5750 Author: pbrezina Title: #5750: fix compilation warnings Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5750/head:pr5750 git checkout pr5750
sssd-devel@lists.fedorahosted.org
-
alexey-tikhonov -
elkoniu
-
pbrezina