GCC 8 Compiler Warning Fixes, remove unused function #24249
Conversation
This function has erroneous code that is causing the warning. It also contains other code that appears to be nonsense. The file descriptors passed to FD_SET and FD_ISSET don't match. Not only does nothing in the repository call this, the function as written could not work properly.
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24249/5955 |
|
A new Pull Request was created by @wddgit (W. David Dagenhart) for master. It involves the following packages: IORawData/DTCommissioning @arunhep, @tocheng, @cmsbuild, @franzoni, @pohsun, @lpernie can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
Hello @wddgit , are you sure we can simply remove WaitData? Maybe there is some other workflow we should test? |
|
As far as I can see there is absolutely nothing in the CMSSW repository that calls this function (based on a text search with LXR). So there is no workflow that could test it unless there is a test outside the CMSSW repository (or something really really bizarre). After spending time reading documentation about the select function, I came to the conclusion that this function is not even properly coded. Even if someone tried to use it, it looks broken to me. |
|
Maybe we can go ahead with this one and merge it. We've given the experts a week and no response, probably because nobody uses or cares about the function being deleted. In the worst case, something outside the repository will fail to compile when it is upgraded to a release containing this change. It is much more likely nothing uses it anywhere and no one will notice. The function appears to me to be incorrectly written in 3 ways. I am not sure, but I doubt it would do what the author originally intended, so it would be good to draw their attention to it in the unlikely case it is actually being used. It is easy to resurrect code and bring it back if necessary. |
|
@lpernie @wddgit a lack of response could be also linked to the vacation period. Anyway, if nothing in the release is sensitive to this function, I would suggest to move forward, private could could be adjusted afterwards (and anyway if the removed code is buggy it would be a good opportunity to review it) |
|
Just for the record in case someone comes back to fix this, the 3 problems I see (from reading documentation, I'd never heard of the 'select' function before trying to fix this compiler warning):
|
|
+1 |
cmsbuild
added
alca-approved
fully-signed
and removed
alca-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
This function has erroneous code that is causing the warning.
It also contains other code that appears to be nonsense.
The file descriptors passed to FD_SET and FD_ISSET don't
match. Not only does nothing in the repository call this,
the function as written could not work properly.