[new Warnings policy] what to do about msvc "deprecation" and "unsafe" warnings
I've implemented the suggestions in "new warnings policy". It probably DID catch a few bugs which have probably been in there for years. It wasn't as bad as I thought - mostly tedium and a few hours of testing. But now I have a couple of questions: a) I added <toolset>gcc:<warnings>all # ? <toolset>msvc:<warnings>all # == /W4 To my jamfile. With MSVC 9.0 I now get a blizzard of "deprecation" and "unsafe" warnings. If the policy is updated to address this, I will implement the recommendations. Robert Ramey PS I'm wondering if I should replace the above with something which maximizes the warning level for all compiles. Perhaps just <warnings>all #?
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Robert Ramey Sent: Monday, November 16, 2009 5:56 AM To: boost@lists.boost.org Subject: [boost] [new Warnings policy] what to do about msvc "deprecation" and "unsafe" warnings
I've implemented the suggestions in "new warnings policy". It probably DID catch a few bugs which have probably been in there for years. It wasn't as bad as I thought - mostly tedium and a few hours of testing. But now I have a couple of questions:
a) I added
<toolset>gcc:<warnings>all # ? <toolset>msvc:<warnings>all # == /W4
To my jamfile.
With MSVC 9.0 I now get a blizzard of "deprecation" and "unsafe" warnings. If the policy is updated to address this, I will implement the recommendations.
Robert Ramey
PS
I'm wondering if I should replace the above with something which maximizes the warning level for all compiles. Perhaps just <warnings>all #?
Would adding the MSVC *_SECURE_* defines to NOT deprecate the 'insecure' version help? project : requirements # The define of macros below prevent warnings about the checked versions of SCL and CRT libraries. # Most Boost code does not need these versions (as they are markedly slower). <toolset>msvc:<define>_SCL_SECURE_NO_WARNINGS <toolset>msvc:<define>_SCL_SECURE_NO_DEPRECATE <toolset>msvc:<define>_CRT_SECURE_NO_WARNINGS <toolset>msvc:<define>_CRT_SECURE_NO_DEPRECATE # Alternatively, you can just suppress the warnings (probably not the best way). <toolset>msvc:<cxxflags>/wd4996 # 'putenv': The POSIX name for this item is deprecated. (and other similar deprecations). ; As suggested in https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines This may also deal with the "unsafe"? Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
First, I have to say, amazing job, and amazing reference. I don't believe that there's anything else like this on the web. Now just some thoughts. N.B. my assumption is that getting rid of the warning with code changes that don't obfuscate meaning is preferred to suppressing. Suggestion column: I would prefer that the this column tell me how to write my code differently to avoid the warning if possible. The Suppression column does a good job of telling me how to suppress. Suppression column: Wonderful! C4180 Surely instead of suggesting suppressing you would remove the meaningless qualifier? C4996 Microsoft deprecated these functions on their own (and they call themselves POSIX compliant), and replaced them with extremely dangerous replacements that they claim are safe. The replacement functions are non-POSIX and aren't available from other compilers, hence non-portable. These warnings should be suppressed. I believe that you can also suppress by defining _CRT_NONSTDC_NO_DEPRECATE before and undef'ing after to do a local suppression. C4800 Might suggest that they use a bool valued expression in the first place, i.e. instead of foo, foo!=0, or do a static cast to bool. This is at times indicative of real bugs, when people turn out to not be doing what they thought they were doing. Apparently this is one of my favorite bugs (by favorite I don't mean that I like it either!) C4506 Might suggest that they provide the definition;) Someone had mentioned a redesign of boost:noncopyable to work around the C4511 and C4512 warnings from deriving from boost:noncopyable. Is anyone looking into it? Is it even possible? It's a shame that this wonderful thing generates so much noise. If course, if you want to make a class uncopyable, rather than inheriting from boost:uncopyable, you could just make your own copy and assignment declarations privately and without definitions if you don't need them. It makes the class non-copyable without generating the warnings. If you have other causes of C451{0-2} see below: C4510/C4511/C4512 In general if you provide one of destructor, assignment operator, and copy constructor, then you need all three. The best fix is to provide the method. Providing a private declaration is a way of telling the compiler that you are sure your case is different--if it really is. These are all caused by either inaccessible base class version, or const or reference data members. Similar to 4511 and 4512 are the following which occur on trying to derive from a non-copyable class, i.e. one with the preceding issues--you will also get 4511 and 4512. C4625: copy constructor could not be generated because a base class copy constructor is inaccessible C4626: assignment operator could not be generated because a base class assignment operator is inaccessible C4701 Initialize the variable. (Does it give this warning for volatile? If it does it's a bug in the compiler. But you would need to suppress in that case.) C4702 If the code is really unreachable delete it. C4535 Does it give this warning even when /EHa is used? You mean that they're telling you to use something you are using? Wish I had Visual C++ here right now to try this one. It's a bug against the compiler if so. If /EH is not being used, it will cause memory leaks, so its use should be added to the tests of things that use this. I would suppress only if /EHa is used and warnings still printed.
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Patrick Horgan Sent: Monday, November 16, 2009 9:09 PM To: boost@lists.boost.org Subject: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
First, I have to say, amazing job, and amazing reference. I don't believe that there's anything else like this on the web.
:-) https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines now updated using your suggestions. Is this better? Paul PS The coverage of gcc is thin, for other compilers, non-existent. Contributions welcome as I am entirely ignorant of these compilers.
Paul A. Bristow wrote: Patrick Horgan First, I have to say, amazing job, and amazing reference. I don't believe that there's anything else like this on the web. :-) [1]https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines now updated using your suggestions. Is this better? Hard to be much better than it was! Still a couple of caveats. I'd like to see C4996 mention that POSIX has not deprecated these please. The Visual C++ message is, perhaps, deceptive and people may fear that they are using things that are no longer part of POSIX. C4710 mention that if the variable is volatile this should be suppressed. C4510 mention if have reference or const members provide default constructor with initializer list. If intent is to make non-default-constructable, provide private declaration only. Paul PS The coverage of gcc is thin, for other compilers, non-existent. Contributions welcome as I am entirely ignorant of these compilers. Working on it;) Thanks, Patrick _______________________________________________ Unsubscribe & other changes: [2]http://lists.boost.org/mailman/listinfo.cgi/boo st References 1. https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines 2. http://lists.boost.org/mailman/listinfo.cgi/boost
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Patrick Horgan Sent: Tuesday, November 17, 2009 6:38 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
Hard to be much better than it was! Still a couple of caveats. I'd like to see C4996 mention that POSIX has not deprecated these please. The Visual C++ message is, perhaps, deceptive and people may fear that they are using things that are no longer part of POSIX. C4710 mention that if the variable is volatile this should be suppressed. C4510 mention if have reference or const members provide default constructor with initializer list. If intent is to make non-default-constructable, provide private declaration only.
Done
PS The coverage of gcc is thin, for other compilers, non-existent. Contributions welcome as I am entirely ignorant of these compilers.
Working on it;)
Feel free to edit the page directly - it's a wiki after all! Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
On Tue, Nov 17, 2009 at 4:15 AM, Paul A. Bristow <pbristow@hetp.u-net.com> wrote:
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Patrick Horgan Sent: Monday, November 16, 2009 9:09 PM To: boost@lists.boost.org Subject: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
First, I have to say, amazing job, and amazing reference. I don't believe that there's anything else like this on the web.
:-)
https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines
now updated using your suggestions.
Is this better?
Why not replace all that by "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning" followed by information on how warnings can be suppressed on various compilers? Rationale: A) Users don't care why they don't see warnings. B) Even if they did, if the build log has no warnings it is prohibitively difficult to investigate why that is. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Emil Dotchevski Sent: Tuesday, November 17, 2009 8:15 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guideline
Why not replace all that by "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning" followed by information on how warnings can be suppressed on various compilers?
Rationale:
A) Users don't care why they don't see warnings.
I think they *do* care about code quality, and dealing with warnings makes a contribution (perhaps small).
B) Even if they did, if the build log has no warnings it is prohibitively difficult to investigate why that is.
Sorry, but I'm not clear what part you suggest replacing. Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
On Wed, Nov 18, 2009 at 2:51 AM, Paul A. Bristow <pbristow@hetp.u-net.com> wrote:
Why not replace all that by "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning" followed by information on how warnings can be suppressed on various compilers?
Rationale:
A) Users don't care why they don't see warnings.
I think they *do* care about code quality, and dealing with warnings makes a contribution (perhaps small).
B) Even if they did, if the build log has no warnings it is prohibitively difficult to investigate why that is.
Sorry, but I'm not clear what part you suggest replacing.
I thought that it was clear, I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning." Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On Wed, Nov 18, 2009 at 11:37 AM, Emil Dotchevski <emildotchevski@gmail.com> wrote:
On Wed, Nov 18, 2009 at 2:51 AM, Paul A. Bristow <pbristow@hetp.u-net.com> wrote:
Why not replace all that by "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning" followed by information on how warnings can be suppressed on various compilers?
Rationale:
A) Users don't care why they don't see warnings.
I think they *do* care about code quality, and dealing with warnings makes a contribution (perhaps small).
B) Even if they did, if the build log has no warnings it is prohibitively difficult to investigate why that is.
Sorry, but I'm not clear what part you suggest replacing.
I thought that it was clear, I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning."
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote: I thought that it was clear, I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning." Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see [1]http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*. Emil, judging by the last couple of weeks of discussion, I believe that you are hostile to the idea of spending any time looking at warnings, and think that no good will come from it. If you have to deal with them, because users might not like them, you would rather just suppress the warnings, or tell gcc that yours are system headers so that no warnings will appear. Then you would like a stated policy saying that suppressing is as good as fixing, and you're done. This is how you've fairly consistently come across <grin;>, and I'm just asking if this is indeed your position. Fair? Going farther, in this thread you seem to want to remove any help that people might have gotten from the wiki to understand their warnings and deal with them. Is that what you mean? Patrick References 1. http://svn.boost.org/trac/boost/ticket/3641
AMDG Emil Dotchevski wrote:
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*.
+1. Warnings should be suppressed, but fixing them isn't worth the risk late in the release cycle. In Christ, Steven Watanabe
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: Thursday, November 19, 2009 2:11 AM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
AMDG
Emil Dotchevski wrote:
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*.
+1. Warnings should be suppressed, but fixing them isn't worth the risk late in the release cycle.
Definitely +1 more. Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
Paul A. Bristow wrote:
Steven Watanabe wrote:
Emil Dotchevski wrote:
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*.
+1. Warnings should be suppressed, but fixing them isn't worth the risk late in the release cycle.
Definitely +1 more.
I agree with the caveat that suppressing, rather than fixing, for a release is appropriate, but that suppression should be removed and the warning should be addressed otherwise, if possible, after the release. If a warning can be fixed by changing the code, and the result is reasonable and maintainable, and doesn't violate performance demands, then the warning should be eliminated by changing the code. If those criteria cannot be met, then there should be assertions and/or tests that specifically address any issues raised by the warning to ensure that future platform alterations don't violate assumptions made by the code and flagged by the warning. Only then should the warning be suppressed. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Thu, Nov 19, 2009 at 5:21 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Paul A. Bristow wrote:
Steven Watanabe wrote:
Emil Dotchevski wrote:
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*.
+1. Warnings should be suppressed, but fixing them isn't worth the risk late in the release cycle.
Definitely +1 more.
If a warning can be fixed by changing the code, and the result is reasonable and maintainable, and doesn't violate performance demands, then the warning should be eliminated by changing the code.
Two why questions: - why do you think that changing the code is better? - why do we care what action was taken to remove a warning? Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Thu, Nov 19, 2009 at 5:21 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
If a warning can be fixed by changing the code, and the result is reasonable and maintainable, and doesn't violate performance demands, then the warning should be eliminated by changing the code.
Two why questions:
- why do you think that changing the code is better?
If the warning indicates a real issue in the code, suppressing the warning merely masks the problem. If not, then there's the issue of not being able to suppress the warning, as with GCC, or the additional line noise in the form of preprocessor directives or pragmas.
- why do we care what action was taken to remove a warning?
I don't want problems hidden and I'd prefer that the code not have any more preprocessor/pragma line noise than required. Even if I'm not the maintainer, I know that reducing that line noise means that future changes will be easier for the maintainer. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Steven Watanabe wrote:
AMDG
Emil Dotchevski wrote:
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*.
+1. Warnings should be suppressed, but fixing them isn't worth the risk late in the release cycle.
Right, so they should be addressed early in the release cycle? Bo Persson
Bo Persson wrote:
Steven Watanabe wrote:
AMDG
Emil Dotchevski wrote:
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*.
+1. Warnings should be suppressed, but fixing them isn't worth the risk late in the release cycle.
Right, so they should be addressed early in the release cycle?
No. If this is to be a new "requirement", they should be addressed during trunk testing - not in the release. Robert Ramey
Bo Persson
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Thu, Nov 19, 2009 at 2:14 PM, Robert Ramey <ramey@rrsd.com> wrote:
Bo Persson wrote:
Steven Watanabe wrote:
AMDG
Emil Dotchevski wrote:
Actually, prompted by compile error in Boost Exception on GCC 3.4.5, triggered by a warning "fix" made just prior to release (see http://svn.boost.org/trac/boost/ticket/3641) I think we should ban all last minute changes (including warning "fixes") that are not addressing *bugs*.
+1. Warnings should be suppressed, but fixing them isn't worth the risk late in the release cycle.
Right, so they should be addressed early in the release cycle?
No. If this is to be a new "requirement", they should be addressed during trunk testing - not in the release.
I am not confident that even that is conservative enough. In reality, Boost is used on more platforms than we test. Unless a warning fix also fixes a bug, it has *no* positive effects other than hiding the warning, yet may introduce problems that testing could miss. Therefore, I can't justify any warning fix in code that has already been released to the public. In my opinion the only responsible (though still not bullet-proof) way to address such warnings is to suppress them. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
Unless a warning fix also fixes a bug, it has *no* positive effects other than hiding the warning, yet may introduce problems that testing could miss.
The suppression constructs can rot. Future changes to code may obviate the original need to suppress warnings, while preventing the compiler from warning about new (potential) problems. If the maintainer isn't sufficiently disciplined to stop suppressing warnings while working on changes, and then suppressing them again later, if still necessary, other problems can be hidden. It isn't hard to imagine that discipline be lost or never adopted. Changing code to eliminate warnings avoids the need for suppression constructs and leaves the compiler free to warn when the code is changed in the future. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Emil Dotchevski wrote:
I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning."
I like the simplicity of this statement of purpose, but there's value in educating developers on how to judge whether to fix or suppress a warning and in the information on how to deal with specific warnings and compiler vagaries. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Thu, Nov 19, 2009 at 5:24 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Emil Dotchevski wrote:
I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning."
I like the simplicity of this statement of purpose, but there's value in educating developers on how to judge whether to fix or suppress a warning and in the information on how to deal with specific warnings and compiler vagaries.
If I understand correctly, you are concerned with the possibility of developers suppressing warnings as opposed to "fixing them properly." My wording, which is indeed intended to be complete, specifically seeks to avoid such differentiation. Rather than arguing what is right and what is wrong, we should consider the simple fact that if a Boost developer suppresses a warning in a particular library, there are no reasonable grounds to require the warning to be enabled and the code "fixed". Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Thu, Nov 19, 2009 at 5:24 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Emil Dotchevski wrote:
I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning."
I like the simplicity of this statement of purpose, but there's value in educating developers on how to judge whether to fix or suppress a warning and in the information on how to deal with specific warnings and compiler vagaries.
If I understand correctly, you are concerned with the possibility of developers suppressing warnings as opposed to "fixing them properly."
My wording, which is indeed intended to be complete, specifically seeks to avoid such differentiation. Rather than arguing what is right and what is wrong, we should consider the simple fact that if a Boost developer suppresses a warning in a particular library, there are no reasonable grounds to require the warning to be enabled and the code "fixed".
You seem to make two assumptions: warnings are nuisances that should be ignored and all Boost authors and maintainers have sufficient knowledge to judge whether a warning should be ignored (suppressed). Neither assumption is appropriate. For the guideline to be useful, more is needed. Warnings often indicate real problems. Sometimes they only manifest on a particular platform, revealing a portability issue. Sometimes they indicate that the code doesn't account for a runtime condition, like overflow, which the warning can only suggest as a possibility. Suppressing a warning without altering code may simply mask a problem. The right approach is to determine why the warning occurs, decide whether it is correct in the context, and if so, apply appropriate remediation. If the warning is not correct in the context, only then should it be suppressed. Your statement doesn't say even that much. Because developers don't have the same knowledge, even among Boost developers, Boost should amass information to help them know when a warning is significant and not. That information can show cases in which a warning is legitimate and when it isn't. For the former, there can be help to understand how to change the code portably to account for the problem revealed by the warning. For the latter, there can be information on how to suppress the warning in a portable way. I know that changing code can lead to bugs. Thus, changing code to eliminate a warning might create a bug. That's unfortunate. From a maintenance standpoint, however, I'd prefer to see altered code to a glob of preprocessor and pragma line noise that suppresses a warning in the unchanged code. Testing will reveal the bug. If it doesn't, the testing is insufficient. If the bug appears on an untested platform, then more testers are needed to be able to detect such bugs in the future. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning."
I like the simplicity of this statement of purpose, but there's value in educating developers on how to judge whether to fix or suppress a warning and in the information on how to deal with specific warnings and compiler vagaries.
If I understand correctly, you are concerned with the possibility of developers suppressing warnings as opposed to "fixing them properly."
My wording, which is indeed intended to be complete, specifically seeks to avoid such differentiation. Rather than arguing what is right and what is wrong, we should consider the simple fact that if a Boost developer suppresses a warning in a particular library, there are no reasonable grounds to require the warning to be enabled and the code "fixed".
You seem to make two assumptions: warnings are nuisances that should be ignored and all Boost authors and maintainers have sufficient knowledge to judge whether a warning should be ignored (suppressed). Neither assumption is appropriate. For the guideline to be useful, more is needed.
Warnings often indicate real problems. Sometimes they only manifest on a particular platform, revealing a portability issue. Sometimes they indicate that the code doesn't account for a runtime condition, like overflow, which the warning can only suggest as a possibility. Suppressing a warning without altering code may simply mask a problem. The right approach is to determine why the warning occurs, decide whether it is correct in the context, and if so, apply appropriate remediation. If the warning is not correct in the context, only then should it be suppressed. Your statement doesn't say even that much.
Because developers don't have the same knowledge, even among Boost developers, Boost should amass information to help them know when a warning is significant and not. That information can show cases in which a warning is legitimate and when it isn't. For the former, there can be help to understand how to change the code portably to account for the problem revealed by the warning. For the latter, there can be information on how to suppress the warning in a portable way.
I know that changing code can lead to bugs. Thus, changing code to eliminate a warning might create a bug. That's unfortunate. From a maintenance standpoint, however, I'd prefer to see altered code to a glob of preprocessor and pragma line noise that suppresses a warning in the unchanged code. Testing will reveal the bug. If it doesn't, the testing is insufficient. If the bug appears on an untested platform, then more testers are needed to be able to detect such bugs in the future.
Very elegantly put, and sums up very much how I feel as well. To put it another way, for some users, any warning *is* a bug. John.
(Apologies of losing track of the quoting.)
Warnings often indicate real problems.
In this case, the real problem should be fixed - no matter whether it causes a warning or not on compilers we care about. We already know how to do this. There is no need to have a "warning policy" to fix bugs (I hope!) A "warning policy" is only needed for warnings that do not indicate real problems.
Peter Dimov wrote:
(Apologies of losing track of the quoting.)
Warnings often indicate real problems.
A "warning policy" is only needed for warnings that do not indicate real problems.
That's a naive position. If one is in the habit of ignoring warnings (suppressed or otherwise), one may miss the error. Warnings can alert the developer to a problem before anyone notices it in real use. Consequently, the warnings policy is to build at a reasonably high warning level and to emit none. The latter involves the other things under discussion. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Stewart, Robert wrote:
Peter Dimov wrote:
(Apologies of losing track of the quoting.)
Warnings often indicate real problems.
A "warning policy" is only needed for warnings that do not indicate real problems.
That's a naive position.
Thanks.
If one is in the habit of ignoring warnings (suppressed or otherwise), one may miss the error. Warnings can alert the developer to a problem before anyone notices it in real use. Consequently, the warnings policy is to build at a reasonably high warning level and to emit none.
This is a reasonable warnings policy for a company, or a project, but is not a Boost warnings policy. The Boost warnings policy needs to - state whether a warning is considered a bug, - state whether and how a developer should deal with reports (and trac tickets) that a warning is emitted, - ensure that warnings are seen by developers, - ensure that a warning, once eliminated, stays eliminated, - ensure that warnings are not introduced into other developers's code. Note that if you replace "warning" with "bug", the items above no longer make sense - that's because we already have a policy for bugs and don't need another one.
Peter Dimov wrote:
Stewart, Robert wrote:
If one is in the habit of ignoring warnings (suppressed or otherwise), one may miss the error. Warnings can alert the developer to a problem before anyone notices it in real use. Consequently, the warnings policy is to build at a reasonably high warning level and to emit none.
This is a reasonable warnings policy for a company, or a project, but is not a Boost warnings policy.
I don't understand why you'd make that distinction. Why would a reasonably high warning level not be good Boost policy? Why would emitting no warnings not be good Boost policy?
The Boost warnings policy needs to
- state whether a warning is considered a bug,
I perceive you could mean either of two things by that statement: determine whether to consider all warnings from Boost code seen by users as bugs or list each potential compiler warning and indicate whether it should be considered legitimate or not. I presume you mean the former, as the latter isn't generally possible, though one could, of course, discuss the various circumstances that would trigger particular warnings that should be treated as a bug.
- state whether and how a developer should deal with reports (and trac tickets) that a warning is emitted,
Reasonable.
- ensure that warnings are seen by developers,
I take that to mean a reporting mechanism to ensure that the author/maintainer of a library is alerted to warnings occurring on the trunk.
- ensure that a warning, once eliminated, stays eliminated,
I suppose that you mean to imply the reporting mechanism above and a policy to disallow code so reported in a release. If so, I agree.
- ensure that warnings are not introduced into other developers's code.
I'm not sure quite what you mean or how you would impose this. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Stewart, Robert wrote:
Peter Dimov wrote:
- ensure that warnings are not introduced into other developers's code.
I'm not sure quite what you mean or how you would impose this.
Developer A has written code that compiles cleanly with MSVC /W4. Developer B fixes an issue in this code and the result compiles cleanly with g++ -Wall, but happens to introduce a warning under MSVC. (Or Sun C++, for that matter.) (Incidentally, both developers have built at a reasonably high warning level and have seen no warnings.)
Peter Dimov wrote:
Stewart, Robert wrote:
Peter Dimov wrote:
- ensure that warnings are not introduced into other developers's code.
I'm not sure quite what you mean or how you would impose this.
Developer A has written code that compiles cleanly with MSVC /W4. Developer B fixes an issue in this code and the result compiles cleanly with g++ -Wall, but happens to introduce a warning under MSVC. (Or Sun C++, for that matter.)
(Incidentally, both developers have built at a reasonably high warning level and have seen no warnings.)
It would be reasonable to state that if a developer changes the trunk in such a way as to introduce warnings on an inaccessible platform, that developer is required to seek support from other developers to discover a mutually compatible change. It would be plausible to require only that a developer monitor trunk tests for warnings on other platforms while trying variations of the code seeking compatible changes. However, that would mean a longer cycle time and more noise in the diffs while seeking an appropriate variation. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
On Mon, Nov 23, 2009 at 10:28 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Peter Dimov wrote:
Stewart, Robert wrote:
Peter Dimov wrote:
- ensure that warnings are not introduced into other developers's code.
I'm not sure quite what you mean or how you would impose this.
Developer A has written code that compiles cleanly with MSVC /W4. Developer B fixes an issue in this code and the result compiles cleanly with g++ -Wall, but happens to introduce a warning under MSVC. (Or Sun C++, for that matter.)
(Incidentally, both developers have built at a reasonably high warning level and have seen no warnings.)
It would be reasonable to state that if a developer changes the trunk in such a way as to introduce warnings on an inaccessible platform, that developer is required to seek support from other developers to discover a mutually compatible change.
I'd call this common sense. Assuming you're not proposing that we introduce some kind of bureaucracy or a committee to help manage warnings, and assuming everyone agrees that we don't want warnings in the build logs, then simply reporting a warning will get it removed. Or are you envisioning a different process? Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Mon, Nov 23, 2009 at 10:28 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
It would be reasonable to state that if a developer changes the trunk in such a way as to introduce warnings on an inaccessible platform, that developer is required to seek support from other developers to discover a mutually compatible change.
I'd call this common sense.
In many cases, common sense isn't so common.
Assuming you're not proposing that we introduce some kind of bureaucracy or a committee to help manage warnings, and assuming everyone agrees that we don't want warnings in the build logs, then simply reporting a warning will get it removed.
Being clear about what's expected removes doubt, ambiguity, and confusion.
Or are you envisioning a different process?
I'm envisioning only what I wrote above: a policy statement. _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
Warnings often indicate real problems.
In this case, the real problem should be fixed - no matter whether it causes a warning or not on compilers we care about. We already know how to do this. There is no need to have a "warning policy" to fix bugs (I hope!)
A "warning policy" is only needed for warnings that do not indicate real problems. Something that is sometime lost here, is that just because a particular warning, about a particular line of code, doesn't indicate a bug, i.e. incorrect behavior, that doesn't imply that it's not a problem. (Wow,
Peter Dimov wrote: that's really bad English, but it's ok, I speak Texan most natively.;) Fixing warnings by doing code changes allows you to leave warnings enabled, and that same warning, on that line or another, in the future may very well point you right to a bug. Fixing warnings like that sometimes even result in better code, i.e. more readable, more maintainable, more correct, able to resist subtle bugs, but not always. What they do result in is code that can take advantage in the future of all the aid a compiler can give you in catching problems before they escape out into the wild. That's the real reason for making your code so that it can compile with high warning levels without problems. Of course, as you work through your code and get rid of the warnings you always find real bugs too, as some on this list have recently reported while cleaning up their own code. That's just icing on the cake. Those bugs would have already been found if they had already had warning clean code and built at high warning levels. The quality of their code is now higher, and will stay higher. Going through this process takes sloppy code and tightens it up. Patrick
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of John Maddock Sent: Friday, November 20, 2009 4:17 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on theMaintenance Guidelines
You seem to make two assumptions: warnings are nuisances that should be ignored and all Boost authors and maintainers have sufficient knowledge to judge whether a warning should be ignored (suppressed). Neither assumption is appropriate. For the guideline to be useful, more is needed.
Warnings often indicate real problems. Sometimes they only manifest on a particular platform, revealing a portability issue. Sometimes they indicate that the code doesn't account for a runtime condition, like overflow, which the warning can only suggest as a possibility. Suppressing a warning without altering code may simply mask a problem. The right approach is to determine why the warning occurs, decide whether it is correct in the context, and if so, apply appropriate remediation. If the warning is not correct in the context, only then should it be suppressed. Your statement doesn't say even that much.
Because developers don't have the same knowledge, even among Boost developers, Boost should amass information to help them know when a warning is significant and not. That information can show cases in which a warning is legitimate and when it isn't. For the former, there can be help to understand how to change the code portably to account for the problem revealed by the warning. For the latter, there can be information on how to suppress the warning in a portable way.
I know that changing code can lead to bugs. Thus, changing code to eliminate a warning might create a bug. That's unfortunate. From a maintenance standpoint, however, I'd prefer to see altered code to a glob of preprocessor and pragma line noise that suppresses a warning in the unchanged code. Testing will reveal the bug. If it doesn't, the testing is insufficient. If the bug appears on an untested platform, then more testers are needed to be able to detect such bugs in the future.
Very elegantly put, and sums up very much how I feel as well.
To put it another way, for some users, any warning *is* a bug.
So well put that I have added the essence of these to the https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
On Fri, Nov 20, 2009 at 6:33 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Emil Dotchevski wrote:
On Thu, Nov 19, 2009 at 5:24 AM, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Emil Dotchevski wrote:
I propose to replace all of the existing warnings guidelines in the Wiki with the following: "No warnings should be emitted by Boost code. Use your best judgment to either fix or suppress the warning."
I like the simplicity of this statement of purpose, but there's value in educating developers on how to judge whether to fix or suppress a warning and in the information on how to deal with specific warnings and compiler vagaries.
If I understand correctly, you are concerned with the possibility of developers suppressing warnings as opposed to "fixing them properly."
My wording, which is indeed intended to be complete, specifically seeks to avoid such differentiation. Rather than arguing what is right and what is wrong, we should consider the simple fact that if a Boost developer suppresses a warning in a particular library, there are no reasonable grounds to require the warning to be enabled and the code "fixed".
You seem to make two assumptions: warnings are nuisances that should be ignored and all Boost authors and maintainers have sufficient knowledge to judge whether a warning should be ignored (suppressed).
I don't think I made any of these assumptions. My assumptions are: 1) Boost developers have enough knowledge of C++ programming to understand what a warning says. 2) Boost developers, knowing what the warning says, and having designed the code in question, are best equipped to decide what action should be taken. Obviously, if a warning indicates a bug, the *bug* has to be fixed, but that's just common sense, we don't need a policy for this case. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
On Mon, Nov 16, 2009 at 4:08 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
C4800 Might suggest that they use a bool valued expression in the first place, i.e. instead of foo, foo!=0, or do a static cast to bool. This is at times indicative of real bugs, when people turn out to not be doing what they thought they were doing. Apparently this is one of my favorite bugs (by favorite I don't mean that I like it either!)
C4800: int' : forcing value to bool 'true' or 'false' I'm a big fan of using !! to convert to bool: bool has_item() { Foo * foo = find_item(); return !!foo; // convert ptr to bool } Is that too subtle for others? It wouldn't be too subtle if it became a common idiom. :-) Tony
Gottlob Frege wrote:
On Mon, Nov 16, 2009 at 4:08 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
C4800 Might suggest that they use a bool valued expression in the first place, i.e. instead of foo, foo!=0, or do a static cast to bool. This is at times indicative of real bugs, when people turn out to not be doing what they thought they were doing. Apparently this is one of my favorite bugs (by favorite I don't mean that I like it either!)
C4800: int' : forcing value to bool 'true' or 'false'
I'm a big fan of using !! to convert to bool:
bool has_item() { Foo * foo = find_item();
return !!foo; // convert ptr to bool }
Is that too subtle for others? It wouldn't be too subtle if it became a common idiom. :-)
It looks like a quite recommended idiom (double-bang trick) http://www.artima.com/cppsource/safebool.html Best regards, -- Mateusz Loskot, http://mateusz.loskot.net Charter Member of OSGeo, http://osgeo.org
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Mateusz Loskot Sent: Tuesday, November 17, 2009 3:36 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
Gottlob Frege wrote:
On Mon, Nov 16, 2009 at 4:08 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
C4800 Might suggest that they use a bool valued expression in the first place, i.e. instead of foo, foo!=0, or do a static cast to bool. This is at times indicative of real bugs, when people turn out to not be doing what they thought they were doing. Apparently this is one of my favorite bugs (by favorite I don't mean that I like it either!)
C4800: int' : forcing value to bool 'true' or 'false'
I'm a big fan of using !! to convert to bool:
bool has_item() { Foo * foo = find_item();
return !!foo; // convert ptr to bool }
Is that too subtle for others? It wouldn't be too subtle if it became a common idiom. :-)
It looks like a quite recommended idiom (double-bang trick)
Looks OK to me but anyone 'deprecating' this before I add it to the guidelines? Are we sure that all compilers can evaluate this as compile time - otherwise while debugging users might find it a pest? Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
On 17 Nov 2009, at 16:22, Paul A. Bristow wrote:
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Mateusz Loskot Sent: Tuesday, November 17, 2009 3:36 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
Gottlob Frege wrote:
On Mon, Nov 16, 2009 at 4:08 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
C4800 Might suggest that they use a bool valued expression in the first place, i.e. instead of foo, foo!=0, or do a static cast to bool. This is at times indicative of real bugs, when people turn out to not be doing what they thought they were doing. Apparently this is one of my favorite bugs (by favorite I don't mean that I like it either!)
C4800: int' : forcing value to bool 'true' or 'false'
I'm a big fan of using !! to convert to bool:
bool has_item() { Foo * foo = find_item();
return !!foo; // convert ptr to bool }
Is that too subtle for others? It wouldn't be too subtle if it became a common idiom. :-)
It looks like a quite recommended idiom (double-bang trick)
Looks OK to me but anyone 'deprecating' this before I add it to the guidelines?
Are we sure that all compilers can evaluate this as compile time - otherwise while debugging users might find it a pest?
While this is a a workaround that disables the warning, I personally find it almost unreadable, and wouldn't want to encourage it's usage. In my mind when I see a ! I think "not", and have to always apply extra brain power to realise there are 2, and what it means. Chris
Looks OK to me but anyone 'deprecating' this before I add it to the guidelines?
Are we sure that all compilers can evaluate this as compile time - otherwise while debugging users might find it a pest?
While this is a a workaround that disables the warning, I personally find it almost unreadable, and wouldn't want to encourage it's usage. In my mind when I see a ! I think "not", and have to always apply extra brain power to realise there are 2, and what it means.
Me too. I accept that this idiom has it's uses though - for example where the type being tested has an operator !() but no direct conversion to bool - although the "safe bool conversion idiom" used in Boost and elsewhere is IMO superior to this. What's wrong with using either x != 0, or x == 0, depending on which result you want? It's pretty explicit , and exactly mirrors what the compiler has to do internally to convert x to a bool in the first place? John .
John Maddock wrote:
Looks OK to me but anyone 'deprecating' this before I add it to the guidelines?
Are we sure that all compilers can evaluate this as compile time - otherwise while debugging users might find it a pest?
While this is a a workaround that disables the warning, I personally find it almost unreadable, and wouldn't want to encourage it's usage. In my mind when I see a ! I think "not", and have to always apply extra brain power to realise there are 2, and what it means.
Me too.
And me too. However due to coding styles I've been using I'm getting used to it.
I accept that this idiom has it's uses though - for example where the type being tested has an operator !() but no direct conversion to bool - although the "safe bool conversion idiom" used in Boost and elsewhere is IMO superior to this.
What's wrong with using either x != 0, or x == 0, depending on which result you want? It's pretty explicit , and exactly mirrors what the compiler has to do internally to convert x to a bool in the first place?
Nothing wrong for me personally, but I've heard some say it's too long and unnecessary as in assert(0 != ptr) all 5 characters can be replaced with just one ! I prefer explicit version - code for people first, then for computers. Best regards -- Mateusz Loskot, http://mateusz.loskot.net Charter Member of OSGeo, http://osgeo.org
On Tue, Nov 17, 2009 at 2:57 PM, Mateusz Loskot <mateusz@loskot.net> wrote:
Nothing wrong for me personally, but I've heard some say it's too long and unnecessary as in assert(0 != ptr) all 5 characters can be replaced with just one !
Assuming ptr is a pointer, AFAIK assert(ptr) is illegal, though assert(ptr!=0) and assert(!ptr) are OK. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Tue, Nov 17, 2009 at 2:57 PM, Mateusz Loskot <mateusz@loskot.net> wrote:
Nothing wrong for me personally, but I've heard some say it's too long and unnecessary as in assert(0 != ptr) all 5 characters can be replaced with just one !
Assuming ptr is a pointer, AFAIK assert(ptr) is illegal, though assert(ptr!=0) and assert(!ptr) are OK.
Yes, that's right. In this case some suggest to use assert(!!ptr) what I do not really consider personally as readable, prefering assert(0 != ptr); Best regards, -- Mateusz Loskot, http://mateusz.loskot.net Charter Member of OSGeo, http://osgeo.org
On Tue, Nov 17, 2009 at 7:35 PM, Mateusz Loskot <mateusz@loskot.net> wrote:
Emil Dotchevski wrote:
On Tue, Nov 17, 2009 at 2:57 PM, Mateusz Loskot <mateusz@loskot.net> wrote:
Nothing wrong for me personally, but I've heard some say it's too long and unnecessary as in assert(0 != ptr) all 5 characters can be replaced with just one !
Assuming ptr is a pointer, AFAIK assert(ptr) is illegal, though assert(ptr!=0) and assert(!ptr) are OK.
Yes, that's right. In this case some suggest to use assert(!!ptr) what I do not really consider personally as readable, prefering assert(0 != ptr);
Can someone explain how assert(ptr) is illegal? I haven't heard that before. Is if (ptr) illegal now too? Tony
AMDG Gottlob Frege wrote:
On Tue, Nov 17, 2009 at 7:35 PM, Mateusz Loskot <mateusz@loskot.net> wrote:
Emil Dotchevski wrote:
Assuming ptr is a pointer, AFAIK assert(ptr) is illegal, though assert(ptr!=0) and assert(!ptr) are OK.
Yes, that's right. In this case some suggest to use assert(!!ptr) what I do not really consider personally as readable, prefering assert(0 != ptr);
Can someone explain how assert(ptr) is illegal? I haven't heard that before. Is if (ptr) illegal now too?
I don't think so. I don't have a copy of the current C standard handy, but n1336 (which is the current draft) says: "Arithmetic types and pointer types are collectively called scalar types. Array and structure types are collectively called aggregate types.39)" "#include <assert.h> void assert(scalar expression);" In Christ, Steven Watanabe
Paul A. Bristow wrote: -----Original Message----- From: [1]boost-bounces@lists.boost.org [[2]mailto:boost-bounces@lists.boost.org ] On Behalf Of Mateusz Loskot Sent: Tuesday, November 17, 2009 3:36 PM To: [3]boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines Gottlob Frege wrote: On Mon, Nov 16, 2009 at 4:08 PM, Patrick Horgan [4]<phorgan1@gmail.com> wrote: C4800 Might suggest that they use a bool valued expression in the first place, i.e. instead of foo, foo!=0, or do a static cast to bool. This is at times indicative of real bugs, when people turn out to not be doing what they thought they were doing. Apparently this is one of my favorite bugs (by favorite I don't mean that I like it either!) C4800: int' : forcing value to bool 'true' or 'false' I'm a big fan of using !! to convert to bool: bool has_item() { Foo * foo = find_item(); return !!foo; // convert ptr to bool } Is that too subtle for others? It wouldn't be too subtle if it became a common idiom. :-) It looks like a quite recommended idiom (double-bang trick) [5]http://www.artima.com/cppsource/safebool.html Looks OK to me but anyone 'deprecating' this before I add it to the guidelines? For signed integral types, using !!val folds all the negative and positive non-zero values into true. It's the equivalent of val!=0. Sometimes, people really mean to test for val!=0, sometimes they really mean val>0. !!val will still be true even if val is -42. Why not say what you mean? It's communicates better and leads to less subtle bugs. Use one of val!=0, val>0, or whatever test you really mean to do. I understand !!val is the same as val!=0 when I read it and hope that the original programmer also understood that. When I maintain someone else's code, and see val!=0 I feel more happy fuzzies that they said what they meant. Patrick References 1. mailto:boost-bounces@lists.boost.org 2. mailto:boost-bounces@lists.boost.org 3. mailto:boost@lists.boost.org 4. mailto:phorgan1@gmail.com 5. http://www.artima.com/cppsource/safebool.html
2009/11/17 Patrick Horgan <phorgan1@gmail.com>:
For signed integral types, using !!val folds all the negative and positive non-zero values into true. It's the equivalent of val!=0. Sometimes, people really mean to test for val!=0, sometimes they really mean val>0. !!val will still be true even if val is -42. Why not say what you mean? It's communicates better and leads to less subtle bugs. Use one of val!=0, val>0, or whatever test you really mean to do. I understand !!val is the same as val!=0 when I read it and hope that the original programmer also understood that. When I maintain someone else's code, and see val!=0 I feel more happy fuzzies that they said what they meant.
It's worth noting that if the warning turns up in generic code, you should make sure your workaround doesn't add additional requirements to the type being tested. Daniel
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Patrick Horgan Sent: Tuesday, November 17, 2009 7:59 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
I've tried to distill the many helpful comments on this as follows: ||C4800|| int' : forcing value to bool 'true' or 'false'|| Use a bool valued expression. Write out expressions, for example: "value >= 0" or "ptr != 0" (Boost prefers clarity to curtness). Take care if using templates that constants are of the right type, for example you may need "static_cast<T>(1)". Or use static_cast<bool>(expression). Or suppress. ||# pragma warning(disable: 4800) // int' : forcing value to bool 'true' or 'false' Hope this is better. I note that this will mean quite a lot of work for some people as "if (ptr) ..." is such a common (and IMO dreadful) idiom. So I've left the getout clause "suppress" as a last resort. But this might encourage people to write new code in a nicer way. Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
Paul A. Bristow wrote:
||# pragma warning(disable: 4800) // int' : forcing value to bool 'true' or 'false'
Hope this is better.
I note that this will mean quite a lot of work for some people as "if (ptr) ..." is such a common (and IMO dreadful) idiom.
IIRC the warning isn't generated in that context. -- Daniel Wallin BoostPro Computing http://www.boostpro.com
Paul A. Bristow wrote:
I note that this will mean quite a lot of work for some people as "if (ptr) ..." is such a common (and IMO dreadful) idiom.
What in the world is so dreadful about that? _____ Rob Stewart robert.stewart@sig.com Software Engineer, Core Software using std::disclaimer; Susquehanna International Group, LLP http://www.sig.com IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
AMDG Paul A. Bristow wrote:
Guidelines
I've tried to distill the many helpful comments on this as follows:
||C4800|| int' : forcing value to bool 'true' or 'false'||
Use a bool valued expression.
Write out expressions, for example: "value >= 0" or "ptr != 0" (Boost prefers clarity to curtness). Take care if using templates that constants are of the right type, for example you may need "static_cast<T>(1)". Or use static_cast<bool>(expression). Or suppress.
||# pragma warning(disable: 4800) // int' : forcing value to bool 'true' or 'false'
Hope this is better.
I note that this will mean quite a lot of work for some people as "if (ptr) ..." is such a common (and IMO dreadful) idiom.
So I've left the getout clause "suppress" as a last resort.
But this might encourage people to write new code in a nicer way.
I think the most common cause of this warning is that std::type_info::before (which is supposed to return bool) returns int instead. Also, static_cast<bool> doesn't suppress the warning. In Christ, Steven Watanabe
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: Wednesday, November 18, 2009 3:10 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
AMDG
I think the most common cause of this warning is that std::type_info::before (which is supposed to return bool) returns int instead.
Also, static_cast<bool> doesn't suppress the warning.
So is the final suggestion "suppress" correct in this case? I note boost/mpi/detail/mpi_datatype_cache.hpp Matthias Troyer writes (complains!) // The std::type_info::before function in Visual C++ 8.0 (and probably earlier) // incorrectly returns an "int" instead of a "bool". Then the compiler has the // audacity to complain when that "int" is converted to a "bool". Silence // this warning. #ifdef BOOST_MSVC # pragma warning(push) # pragma warning(disable : 4800) #endif Or do what? Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
2009/11/18 Paul A. Bristow <pbristow@hetp.u-net.com>:
I note that this will mean quite a lot of work for some people as "if (ptr) ..." is such a common (and IMO dreadful) idiom.
What do you propose as a superior idiom that will work with pointers, smart pointers, optional, ...? Slapping double-exclamation points everywhere is no better.
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Scott McMurray Sent: Wednesday, November 18, 2009 3:19 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
2009/11/18 Paul A. Bristow <pbristow@hetp.u-net.com>:
I note that this will mean quite a lot of work for some people as "if (ptr)
..."
is such a common (and IMO dreadful) idiom.
What do you propose as a superior idiom that will work with pointers, smart pointers, optional, ...?
Slapping double-exclamation points everywhere is no better.
!! is not mentioned in the draft Guidelines, following feedback. The 'safe bool' idiom http://www.artima.com/cppsource/safebool.html might be, but that may be getting into too much detail. I am proposing if (prt != 0) { ...// do something with pointer as it is meaningful. } As several people have observed, this expresses exactly what you mean, showing that prt == 0 is a special case, rather than saving a few keystrokes. It may be obvious for pointers, but having an 'invalid or special' value is used widely elsewhere. Or if that is considered too much work, suppressing the warning (locally to that module). Let's not flagellate ourselves too much over this ;-) Paul --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
Paul A. Bristow wrote:
Hope this is better.
I note that this will mean quite a lot of work for some people as "if (ptr) ..." is such a common (and IMO dreadful) idiom.
So I've left the getout clause "suppress" as a last resort.
But this might encourage people to write new code in a nicer way.
Thank you:) Patrick
On Mon, Nov 16, 2009 at 3:08 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
Wonderful!
C4180 Surely instead of suggesting suppressing you would remove the meaningless qualifier?
C4996 Microsoft deprecated these functions on their own (and they call
themselves POSIX compliant), and replaced them with extremely dangerous replacements that they claim are safe. The replacement functions are non-POSIX and aren't available from other compilers, hence non-portable. These warnings should be suppressed. I believe that you can also suppress by defining _CRT_NONSTDC_NO_DEPRECATE before and undef'ing after to do a local suppression.
Somewhat off topic, but why exactly are these extremely dangerous, and not secure?
C4800 Might suggest that they use a bool valued expression in the first place, i.e. instead of foo, foo!=0, or do a static cast to bool. This is at times indicative of real bugs, when people turn out to not be doing what they thought they were doing. Apparently this is one of my favorite bugs (by favorite I don't mean that I like it either!) C4506 Might suggest that they provide the definition;) Someone had mentioned a redesign of boost:noncopyable to work around the C4511 and C4512 warnings from deriving from boost:noncopyable. Is anyone looking into it? Is it even possible? It's a shame that this wonderful thing generates so much noise. If course, if you want to make a class uncopyable, rather than inheriting from boost:uncopyable, you could just make your own copy and assignment declarations privately and without definitions if you don't need them. It makes the class non-copyable without generating the warnings. If you have other causes of C451{0-2} see below: C4510/C4511/C4512 In general if you provide one of destructor, assignment operator, and copy constructor, then you need all three. The best fix is to provide the method. Providing a private declaration is a way of telling the compiler that you are sure your case is different--if it really is. These are all caused by either inaccessible base class version, or const or reference data members. Similar to 4511 and 4512 are the following which occur on trying to derive from a non-copyable class, i.e. one with the preceding issues--you will also get 4511 and 4512.
Beman proposed changing noncopyable idiom to a macro. This would solve not only the warning problems but also the problem of horrible compiler error messages that results from the current idiom. I would welcome this change. Then we could mark the boost::noncopyable class as deprecated Zach
Zachary Turner wrote:
On Mon, Nov 16, 2009 at 3:08 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
... elision by Patrick ... C4996 Microsoft deprecated these functions on their own (and they call
themselves POSIX compliant), and replaced them with extremely dangerous replacements that they claim are safe. The replacement functions are non-POSIX and aren't available from other compilers, hence non-portable. These warnings should be suppressed. I believe that you can also suppress by defining _CRT_NONSTDC_NO_DEPRECATE before and undef'ing after to do a local suppression.
Somewhat off topic, but why exactly are these extremely dangerous, and not secure?
Sorry about the delay in responding to this, but Nick Stoughton says it much better than I do, so I was waiting for him to get back to me and let me know if I could just quote him on it. Nicely enough, he did, so the following is from him, it's the original liaison memo from POSIX to C when C was considering a proposal to add these interfaces into the lib: Nick Stoughton wrote:
SC 22/WG 14 N1160 Austin Group Concerns on PDTR 24731 Stoughton 2006-02-27 Members of the Austin Group have been reviewing the proposed Technical Report on "Bounds Checking Functions" over the last year, and wish to express their concerns over its direction. The proposed interfaces fail to address many of the aspects related to buffer overflow and as a result are only suitable for a narrow range of applications. The basic idea embodied by the proposed interface is not a new one. For example, the proposed strcpy_s function is similar to the strlcpy function of OpenBSD 2.4 (1998). However, the basic idea has not achieved practical consensus; on the contrary, for reasons discussed below it has been controversial almost since it was introduced. A Technical Report of type 2 does not seem warranted here: the subject is controversial rather than being under technical development, and mere publication of a TR is unlikely to further consensus. The core of the problem is that memory handling in C is complicated and error prone. Nobody doubts that improvements in the supporting APIs are useful. However, the existing APIs already provide all the means to write correct programs, although it is often cumbersome to do so. The proposed interfaces don't change that and, to the contrary, can make programs even more complex. A better solution would be to take the memory handling off the hands of the programmer as much as possible. Let's look at the string functions first. Obviously, code like void f(char *t, const char *s1, const char *s2) { strcpy(t, s1); strcat(t, s2); } is bad. But it is not written this way because it is impossible to write correct code. Obviously the length of the target buffer can be passed, though this alters the ABI (see below): void f(char *t, size_t tlen, const char *s1, const char *s2) { if (strlen(s1)+strlen(s2) >= tlen) abort(); strcpy(t, s1); strcat(t, s2); } This is cumbersome to write and slow which is why programmers don't do it. But, more importantly, this kind of change cannot retroactively made because it changes both the API and ABI. New interfaces would have to be introduced (give the new function a different name) but then one might as well write a better function than this. If f() is in a third party library, it cannot change without all the customers of the library changing and recompiling/relinking their applications. The version using the proposed interfaces has exactly the same problem. void f(char *t, rsize_t tlen, const char *s1, const char *s2) { if (strcpy_s(t, tlen, s1) != 0 || strcat_s(t, tlen, s2) != 0) abort(); } If anything, this code is even less obvious then the previous version even though it is likely a bit faster. Even using the exception handler to provide the abort requires the ABI to change: void f(char *t, rsize_t tlen, const char *s1, const char *s2) { (void) set_constraint_handler_s(abort_handler_s);
Page 1 of 4
strcpy_s(t, tlen, s1); strcat_s(t, tlen, s2); } Another problem with respect to the string functions being used to fix up existing code can be demonstrated with this code sequence: char *p = malloc (3 * NAME_LEN); strcpy (p, name1); strcat (p, name2); strcat (p, name3); All too often fixed values like NAME_LEN are used which are the basis for overflows. A programmer could certainly use char *p = malloc(strlen(name1) + strlen(name2) + strlen(name3) + 1); if (p != NULL) { strcpy (p, name1); strcat (p, name2); strcat (p, name3); } but this is once again cumbersome and therefore won't be used. Now assume the new string functions. The code to correctly handle the code (more correct than either of the previous two code sequences, this is the goal) could look something like this: rsize_t len = 3 * NAME_LEN; char *p = NULL; again: char *p2 = realloc(p, len); if (p2 == NULL) abort (); p = p2; if (strcpy_s(p, len, name1) != 0 || strcat_s(p, len, name2) != 0 || strcat_s(p, len name3) != 0) { len += 2; goto again; } Nobody can say that this is more appealing to the programmer and it is unlikely that code like this will find its way into many programs. Once again, this can be written more simply as char *p = malloc(3*NAME_LEN); set_constraint_handler_s(abort_handler_s); strcpy_s(p, NAME_LEN, name1); strcat_s(p, strlen(p)+NAME_LEN, name2); strcat_s(p, strlen(p)+NAME_LEN, name3); but this still has several vulnerabilities and coding weaknesses, resulting in different end results for p than previously correctly functioning code: 1. Is it certain that name1, name2 and name3 really were a maximum of NAMELEN on entry to this code? What if name1 was NAME_LEN+10, while name2 was NAME_LEN-10? 2. The code needs to be aware of the exception handler; if the handler is not the abort handler, the code *should* check the return code and take appropriate steps if it is to function correctly. A simple drop in replacement as above will fail with the ignore-handler.
Page 2 of 4
The new string functions are meant as an aid to secure existing code bases but the requirement to change ABI (new function parameters, additional elements in structures, etc) plus the added complexity of the code makes the adoption of these interfaces higher unlikely. Instead, a better approach is to eliminate the requirement on the programmer to deal with the allocation him/herself. The runtime should do this. In this users of systems with the GNU C library can simply use char *p; if (asprintf(&p, "%s%s%s", name1, name2, name2) == -1) abort(); The runtime makes sure the target string is large enough and that error conditions (out-of-memory etc) are recognized. It does leave the programmer the responsibility of adding free(p); when he/she is finished with the result, but is this really harder than some of the contortions necessary to use the proposed interfaces? The concept of rsize_t and RSIZE_MAX also cause confusion. The lesson learned from "640 KiB is enough address space" is that there is no fixed limit which people wouldn't want to see lifted over time. On 32-bit systems we used to have 2 GiB or up to 3 GiB of address space available for user-level code. Nowadays the whole 4 GiB is available because people asked for it. Any RSIZE_MAX chosen for 2 or 3 GiB address spaces would prevent using 4 GiB address spaces. The same is true for any other limit and it will definitely remain true for 64-bit architectures as well. Any interface introduced solely for the purpose of using rsize_t instead of size_t is completely unnecessary. Aside from the problem of picking a size, using rsize_t for different sizes like tmpnam_s (for a string) wcscpy_s (a wide char string) and qsort_s (a number of element and a type size) makes no sense. How can a function reject handling strings of, say, 2^20 bytes but allow wcscpy_s to handling 2^22 bytes (on platforms with 4 byte wchar_t)? All of this functionality can be implemented with the existing implementation. It is always possible for the runtime to determine the maximum possible string length, for example, by looking at the gaps in the address space at startup time. This *dynamically* determined value can then be used for sanity checks; no correct program can ever use larger values. It is therefore no violation of the ISO C to handle these situations as error cases. Then there are the stdio and string functions which are now supposed to gracefully handle NULL pointers. However, as "drop-in replacements" for the original functions, these are likely to lead to security weaknesses in the application where the unmodified program would crash. Programmers far too often don't check for errors and so NULL pointers and the like are used in places where they shouldn't. Now assume that the stream pointer is supposed to be for a stream where security logs are written to. If an attacker can overwrite the FILE* value with NULL no more output happens and security problems remain unreported and undetected. The only valid exception handler in these cases should be the abort_handler. There are a myriad of situations like this where unreported invalid input can cause problems. There is a problem with the special handling of printf("%s", NULL) in the GNU C library, which chose to print the string "(null)". In hindsight, this extension probably was not a good idea because it hides problems. The only reasonable way to handle invalid inputs is by brute force: abort the program or at the very least make absolutely sure the user notices the problem. A Denial of Service attack is much less severe and easier to notice and battle than an attack which causes, for instance,
Page 3 of 4
logging to be disabled. The "drop-in" replacement technique introduces subtle semantic changes in the way an application will behave under a certain set of input data, and these semantic changes may well have unintended side effects that make the resulting program less "secure". As a result, any programmer altering an application to make changes to the interfaces to use these bounds-checking versions *must* do more than simply "drop-in" the new version. Since he/she is going to that much work anyway, moving to a paradigm using malloc'ed memory would be no harder, and is demonstrably safer. Another problem can arise in cases where the programmer makes incorrect assumptions about buffer sizes. In this case, the programmer believes that he/she has mitigated all buffer overflow problems by using the new interfaces, but in reality there are still buffer overflows possible in the code. We are not aware of any study demonstrating that the proposed approach leads to safer or more-secure software. On the contrary, when one of us very-briefly attempted to investigate the matter in 2002, he found that the technique (as embodied by the similar strlcpy/strlcat functions of OpenBSD) made C code harder to read and to maintain, while not catching any bugs in the code surveyed. The code surveyed was OpenSSH_3.0.2p1, the then-current version. For some details please see: http://sources.redhat.com/ml/libc-alpha/2002-01/msg00096.html http://sources.redhat.com/ml/libc-alpha/2002-01/msg00159.html In summary, there is no aspect of the proposal which is worth standardizing. Either no change is needed for the new interfaces or there is no chance that the interfaces will find widespread adoption.
Page 4 of 4
On Mon, Nov 23, 2009 at 3:24 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
Zachary Turner wrote:
On Mon, Nov 16, 2009 at 3:08 PM, Patrick Horgan <phorgan1@gmail.com> wrote:
... elision by Patrick ...
C4996 Microsoft deprecated these functions on their own (and they call
themselves POSIX compliant), and replaced them with extremely dangerous replacements that they claim are safe. The replacement functions are non-POSIX and aren't available from other compilers, hence non-portable. These warnings should be suppressed. I believe that you can also suppress by defining _CRT_NONSTDC_NO_DEPRECATE before and undef'ing after to do a local suppression.
Somewhat off topic, but why exactly are these extremely dangerous, and not secure?
Sorry about the delay in responding to this, but Nick Stoughton says it much better than I do, so I was waiting for him to get back to me and let me know if I could just quote him on it. Nicely enough, he did, so the following is from him, it's the original liaison memo from POSIX to C when C was considering a proposal to add these interfaces into the lib:
Nick Stoughton wrote:
SC 22/WG 14 N1160 Austin Group Concerns on PDTR 24731 Stoughton 2006-02-27 Members of the Austin Group have been reviewing the proposed Technical Report on "Bounds Checking Functions" over the last year, and wish to express their concerns over its direction.
... snipped for brevity What follows after the snipped part I find to be a rather strange argument. I mean I get that there's added complexity and that added complexity is "bad", but the proposed "solution" requires that the programmer be ok with the memory being allocated by the CRT, which is grossly limiting and kind of against everything that C / C++ is about. Zach
Zachary Turner wrote:
... elision by Patrick ... What follows after the snipped part I find to be a rather strange argument. I mean I get that there's added complexity and that added complexity is "bad", but the proposed "solution" requires that the programmer be ok with the memory being allocated by the CRT, which is grossly limiting and kind of against everything that C / C++ is about.
I saw them throw that idea out but didn't read it as a proposal, nor an argument, did you really? It read to me as if they were saying, here's something that says it fixes a problem, but it doesn't really, if you changed it like this it would, but that was just to illustrate why the original wouldn't work, not to propose the other. I'm quite sure that the POSIX position was to leave things unchanged. Interesting. I'll have to go back and re-read it. It hadn't occurred to me to read it that way at all. Patrick
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Patrick Horgan Sent: Monday, November 23, 2009 9:24 PM To: boost@lists.boost.org Subject: Re: [boost] [new Warnings policy] MS C4180 on the Maintenance Guidelines
Nick Stoughton wrote:
SC 22/WG 14 N1160 Austin Group Concerns on PDTR 24731 Stoughton 2006-02-27 Members of the Austin Group have been reviewing the proposed Technical Report on "Bounds Checking Functions" over the last year, and wish to express their concerns over its direction. The proposed interfaces fail to address many of the aspects related to buffer overflow and as a result are only suitable for a narrow range of applications.
I've added a link to this at https://svn.boost.org/trac/boost/wiki/Guidelines/MaintenanceGuidelines at the C4996 notes I conclude that people must be allowed make up their own minds about whether to use secure or not, and so should suppress the warnings. Paul PS It reminds me what a disastrous mistake C made when not including an array length as an integral part of the array, leaving checks (perhaps optional) to the compiler (perhaps using hardware to avoid any perceptible runtime cost). The whole virus fiasco can be traced to this. --- Paul A. Bristow Prizet Farmhouse Kendal, UK LA8 8AB +44 1539 561830, mobile +44 7714330204 pbristow@hetp.u-net.com
participants (16)
-
Bo Persson -
Christopher Jefferson -
Daniel James -
Daniel Wallin -
Emil Dotchevski -
Gottlob Frege -
John Maddock -
Mateusz Loskot -
Patrick Horgan -
Paul A. Bristow -
Peter Dimov -
Robert Ramey -
Scott McMurray -
Steven Watanabe -
Stewart, Robert -
Zachary Turner