Re: [webkit-dev] unsigned vs unsigned int
On Sunday 16 September 2012, Darin Adler wrote: On Sep 16, 2012, at 2:30 AM, Allan Sandfeld Jensen k...@carewolf.com wrote: On Thursday 13 September 2012, Dan Bernstein wrote: On Sep 13, 2012, at 1:29 AM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: I was telling people to use unsigned instead of unsigned int, as I have been told that was the preferred style before, but apparently that is not in the style guide. The question is, should it be? Yes. Why? Wouldn't it be better to move away from deprecated C syntax? I think we should use the shortest name for each type. That is why I like “unsigned” better than “unsigned int”. Does this mean you also prefer 'const' over 'const int'? ;-) Anyway, I have no strong opinion on the subject, like most programmers I just prefer what I am just to, and for some reason I am more used to 'unsigned int' than 'unsigned'. But I can easily live with it. This was only meant as a weekend comment. Best regards. `Allan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] unsigned vs unsigned int
On Sep 17, 2012, at 1:22 AM, Allan Sandfeld Jensen k...@carewolf.com wrote: On Sunday 16 September 2012, Darin Adler wrote: I think we should use the shortest name for each type. That is why I like “unsigned” better than “unsigned int”. Does this mean you also prefer 'const' over 'const int'? No. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] How to collect usage metrics for deprecated DOM APIs
I recently added FeatureObserver, which makes it easier to measure how often web pages use deprecated DOM APIs. Here's how to use FeatureObserver: Step 1: Add an enum value to FeatureObserver.h: http://trac.webkit.org/changeset/128798/trunk/Source/WebCore/page/FeatureObserver.h Step 2: Add the [V8MeasureAs=...] IDL attribute to the DOM API you want to measure: http://trac.webkit.org/changeset/128798/trunk/Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.idl Step 3: There is no step 3. There are, however, some limitations currently. FeatureObserver only works on the main thread and only works in the Chromium port. If you want to measure usage of a feature that exists both on the main thread and in workers, you can use the following pattern: http://trac.webkit.org/changeset/128797/trunk/Source/WebCore/fileapi/WebKitBlobBuilder.cpp. Q: What exactly does FeatureObserver measure? A: Feature observer counts the number of Pages that use a particular feature. If a page calls a DOM API many times, we only count it once per page. We use this approach so we can get a sense of what fraction of pages will be affected when we delete the API. Q: How long does it take to get results? A: We'll get rough data from the Canary channel within the week, but usually we're more interested in data from the Stable channel. Getting high-quality data will take about 12-16 weeks. Q: How can I make this work for my port? A: The main thing you need to do is implement the http://trac.webkit.org/browser/trunk/Source/WebCore/platform/HistogramSupport.h interface. HistogramSupport is already used in a number of places throughout WebCore, so implementing that interface will let you measure a variety of interesting data about how your port uses WebKit. (You might also need to do a little CodeGeneratorJS.pm hacking to wire up the IDL attribute as well.) Q: How will we use this data to decide whether to delete a deprecated API? A: That's a much harder question to answer. If FeatureObserver sees very little usage of an API, we might reasonably believe that deleting the API will have little compatibility impact, at least relative to the population we're measuring. The converse, however, is not necessarily true. Hopefully FeatureObserver will give us useful information, but I suspect we will still need to make these sorts of decisions with care. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
Hey folks, TL;DR - If you have opinions one way or another about having a Coverity instance available for WebKit developers, please respond to this message. Coverity is a static analysis tool [1] which scans source code and reports defects in the code. We've been using Coverity to find defects in Chrome for a while now, and though there is sometimes a bit of subjectivity involved in the defect types (e.g. whether a return value should be checked), the signal is generally high. Off the top of my head, the following are the defects I spend most of my time fixing: * Uninitialized variables (including member variables). - Chrome has had at least 4 crash fixes in the past few months due to this defect (which were caught by Coverity). * Passing large parameters by value. - Generally a trivial fix. I don't have performance data to say what affect fixing these hash, but 'death by a thousand cuts' eh? * Forward/Reverse/I - Nulls. - Coverity is very good at understanding when a value is NULL and the tool will tell you which code paths are using a NULL value. * Tons of security issue-causing defects. I'd like to propose adding a Coverity instance for the WebKit community, but I want to make sure there's general support before writing up the detailed proposal. A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). Let me know if you love the idea or hate it. Thanks, James [1] http://www.coverity.com/products/static-analysis.html [2] http://softwareintegrity.coverity.com/coverity-scan-2011-open-source-integrity-report-registration.html - registration required now :( ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
On Mon, Sep 17, 2012 at 4:11 PM, James Hawkins jhawk...@chromium.orgwrote: I'd like to propose adding a Coverity instance for the WebKit community, but I want to make sure there's general support before writing up the detailed proposal. A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). That sounds great to me. On the other hand, I'm strongly opposed to fixing code or rolling out patches just because Coverity warns about it. See http://lists.webkit.org/pipermail/webkit-dev/2012-April/020365.html. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
I wish to subscribe to this product and or service. Count me in. I'm not particularly worried about who has access. But maybe I should be? Its not like the bad-guys can't run coverity themselves. On Mon, Sep 17, 2012 at 6:11 PM, James Hawkins jhawk...@chromium.org wrote: Hey folks, TL;DR - If you have opinions one way or another about having a Coverity instance available for WebKit developers, please respond to this message. Coverity is a static analysis tool [1] which scans source code and reports defects in the code. We've been using Coverity to find defects in Chrome for a while now, and though there is sometimes a bit of subjectivity involved in the defect types (e.g. whether a return value should be checked), the signal is generally high. Off the top of my head, the following are the defects I spend most of my time fixing: * Uninitialized variables (including member variables). - Chrome has had at least 4 crash fixes in the past few months due to this defect (which were caught by Coverity). * Passing large parameters by value. - Generally a trivial fix. I don't have performance data to say what affect fixing these hash, but 'death by a thousand cuts' eh? * Forward/Reverse/I - Nulls. - Coverity is very good at understanding when a value is NULL and the tool will tell you which code paths are using a NULL value. * Tons of security issue-causing defects. I'd like to propose adding a Coverity instance for the WebKit community, but I want to make sure there's general support before writing up the detailed proposal. A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). Let me know if you love the idea or hate it. Thanks, James [1] http://www.coverity.com/products/static-analysis.html [2] http://softwareintegrity.coverity.com/coverity-scan-2011-open-source-integrity-report-registration.html - registration required now :( ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
On Mon, Sep 17, 2012 at 4:11 PM, James Hawkins jhawk...@chromium.orgwrote: A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). I am a little skeptical of Coverity because of bad patches that originated for its report (sometimes even discussed on webkit-dev). I think we should keep in mind the tool also make many mistakes and we should not blindly follows it. Could this be integrated with the EWS like a kind of advanced style check? Reporting possible improvements before patches lands would be more useful than a separate bot. Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
I like having static analysis results available - that seems useful. I think static analysis tools are more useful when we can set a blanket policy of fixing all warnings they report. For example, our warning levels combined with -Werror are a very strong tool in this regard, despite the limited coverage. We can tweak the warning flags to only identify warnings we think outweigh the cost. I am not sure if Coverity can become this type of tool. As others have mentioned, past patches fixing the issues it reports have at times not been net improvements to the code (in that the issue was a false positive, and the resulting code was less readable). Does Coverity have a way to adjust what issues it reports? If so, then starting with a consensus set that the WebKit community agrees are generally worthwhile would make for a very powerful tool. If it does not, then I suspect we could only use it for informational purposes, and not as a get failures down to zero tool, which would significantly diminish the value. We would also have to be clear in that case that coverity reports a problem is not, by itself, a sufficient justification for a code change. With those caveats, I like the idea of making the report available to everyone. At the very least, it would let patch reviewers see Coverity's complaints in context. Regards, Maciej On Sep 17, 2012, at 4:11 PM, James Hawkins jhawk...@chromium.org wrote: Hey folks, TL;DR - If you have opinions one way or another about having a Coverity instance available for WebKit developers, please respond to this message. Coverity is a static analysis tool [1] which scans source code and reports defects in the code. We've been using Coverity to find defects in Chrome for a while now, and though there is sometimes a bit of subjectivity involved in the defect types (e.g. whether a return value should be checked), the signal is generally high. Off the top of my head, the following are the defects I spend most of my time fixing: * Uninitialized variables (including member variables). - Chrome has had at least 4 crash fixes in the past few months due to this defect (which were caught by Coverity). * Passing large parameters by value. - Generally a trivial fix. I don't have performance data to say what affect fixing these hash, but 'death by a thousand cuts' eh? * Forward/Reverse/I - Nulls. - Coverity is very good at understanding when a value is NULL and the tool will tell you which code paths are using a NULL value. * Tons of security issue-causing defects. I'd like to propose adding a Coverity instance for the WebKit community, but I want to make sure there's general support before writing up the detailed proposal. A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). Let me know if you love the idea or hate it. Thanks, James [1] http://www.coverity.com/products/static-analysis.html [2] http://softwareintegrity.coverity.com/coverity-scan-2011-open-source-integrity-report-registration.html - registration required now :( ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
On Mon, Sep 17, 2012 at 6:35 PM, Benjamin Poulain benja...@webkit.org wrote: On Mon, Sep 17, 2012 at 4:11 PM, James Hawkins jhawk...@chromium.org wrote: A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). I am a little skeptical of Coverity because of bad patches that originated for its report (sometimes even discussed on webkit-dev). I think we should keep in mind the tool also make many mistakes and we should not blindly follows it. Could this be integrated with the EWS like a kind of advanced style check? I think this is a great idea, and would be trivial if coverity could be convinced to run on a diff file, or if we could wrap it in a script to only report errors on the changed lines. Either sounds very doable. The EWS infrastructure is already in place once such a script exists. Reporting possible improvements before patches lands would be more useful than a separate bot. Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
On Tue, Sep 18, 2012 at 8:11 AM, James Hawkins jhawk...@chromium.org wrote: Hey folks, TL;DR - If you have opinions one way or another about having a Coverity instance available for WebKit developers, please respond to this message. I don't have an opinion, but: Coverity is a static analysis tool [1] which scans source code and reports defects in the code. We've been using Coverity to find defects in Chrome for a while now, and though there is sometimes a bit of subjectivity involved in the defect types (e.g. whether a return value should be checked), the signal is generally high. Off the top of my head, the following are the defects I spend most of my time fixing: * Uninitialized variables (including member variables). - Chrome has had at least 4 crash fixes in the past few months due to this defect (which were caught by Coverity). This sounds very useful. Do you know how this is done? If you have a class whose constructor calls a clear() function which sets all variables, will it warn about the constructor not initializing all members? If so, how do you suppress the warning in this case? (There was a thread on the clang mailing list on having a warning like this, and we couldn't come up with a good way to handle this case.) * Passing large parameters by value. - Generally a trivial fix. I don't have performance data to say what affect fixing these hash, but 'death by a thousand cuts' eh? I have seen at least three crashes in the last few months that were cause by changes to fix this warning (something that used to be a copied object became a dangling reference). I'm not sure this warning is worth it. * Forward/Reverse/I - Nulls. - Coverity is very good at understanding when a value is NULL and the tool will tell you which code paths are using a NULL value. * Tons of security issue-causing defects. I'd like to propose adding a Coverity instance for the WebKit community, but I want to make sure there's general support before writing up the detailed proposal. A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). Let me know if you love the idea or hate it. Thanks, James [1] http://www.coverity.com/products/static-analysis.html [2] http://softwareintegrity.coverity.com/coverity-scan-2011-open-source-integrity-report-registration.html - registration required now :( ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
On Tue, Sep 18, 2012 at 8:46 AM, Eric Seidel e...@webkit.org wrote: On Mon, Sep 17, 2012 at 6:35 PM, Benjamin Poulain benja...@webkit.org wrote: On Mon, Sep 17, 2012 at 4:11 PM, James Hawkins jhawk...@chromium.org wrote: A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). I am a little skeptical of Coverity because of bad patches that originated for its report (sometimes even discussed on webkit-dev). I think we should keep in mind the tool also make many mistakes and we should not blindly follows it. Could this be integrated with the EWS like a kind of advanced style check? I think this is a great idea, and would be trivial if coverity could be convinced to run on a diff file, or if we could wrap it in a script to only report errors on the changed lines. Either sounds very And/Or are we going to allow inline annotations? The practice Coverity suggested is to adding such annotations. http://scan.coverity.com/best-practice.html I personally think it's worth having inline annotations because it can also help human code readers, so I'm curious what other folks think about that. doable. The EWS infrastructure is already in place once such a script exists. Reporting possible improvements before patches lands would be more useful than a separate bot. Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
On Sep 17, 2012, at 8:13 PM, Hajime Morrita morr...@chromium.org wrote: And/Or are we going to allow inline annotations? The practice Coverity suggested is to adding such annotations. http://scan.coverity.com/best-practice.html I personally think it's worth having inline annotations because it can also help human code readers, so I'm curious what other folks think about that. I hate the idea of mysterious comments to make a tool not complain. Would r-. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Pre-proposal: Adding a Coverity instance for WebKIt
Annotations to spoonfeed a static analysis would make me profoundly unhappy. -Filip On Sep 17, 2012, at 8:13 PM, Hajime Morrita morr...@chromium.org wrote: On Tue, Sep 18, 2012 at 8:46 AM, Eric Seidel e...@webkit.org wrote: On Mon, Sep 17, 2012 at 6:35 PM, Benjamin Poulain benja...@webkit.org wrote: On Mon, Sep 17, 2012 at 4:11 PM, James Hawkins jhawk...@chromium.org wrote: A few details: * Google will front the cost of the license (non-zero...very far from zero) and the infrastructure. * I'd leave it up to the WebKit leadership to decide who has access (most likely limited to WebKit committers for security purposes). The biggest rationale is to provide a strong defect signal for the entire WebKit community, which would directly impact the success of all WebKit-based projects. Coverity has provided free licenses for unsponsored (by larger corporations anyway) open-source projects; this has resulted in significant improvements [2] to the code bases of these projects, one of which I was directly involved with years ago (Wine). I am a little skeptical of Coverity because of bad patches that originated for its report (sometimes even discussed on webkit-dev). I think we should keep in mind the tool also make many mistakes and we should not blindly follows it. Could this be integrated with the EWS like a kind of advanced style check? I think this is a great idea, and would be trivial if coverity could be convinced to run on a diff file, or if we could wrap it in a script to only report errors on the changed lines. Either sounds very And/Or are we going to allow inline annotations? The practice Coverity suggested is to adding such annotations. http://scan.coverity.com/best-practice.html I personally think it's worth having inline annotations because it can also help human code readers, so I'm curious what other folks think about that. doable. The EWS infrastructure is already in place once such a script exists. Reporting possible improvements before patches lands would be more useful than a separate bot. Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
