> - if (cluster_idx >= track->entry) > + /* GCC 5.2 wants to "optimize" cluster_idx >= track->entry to the below > + * expression. We actually mean cluster_idx >= track->entry. */ > + if (cluster_idx - track->entry >= 0) > return 0;
On Sat, Oct 17, 2015 at 11:04 AM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: > Yes, it is really a questions of where on the ROC curve you want to > land, and as such has no technical solution. Since there seems to be > almost universal consensus against my proposal in this case, consider > the patch dropped. I may return to it in a few years :). Just a suggestion but I think the key thing to avoid in future patches is changing an obviously correct check to something else that has the potential to introduce new undefined behavior or subtle bugs. For example the original check here is the obvious check to ensure that access to the cluster array cannot occur at or beyond index track->entry. The proposed replacement cannot ensure that because the arithmetic has the potential to overflow and produce the wrong result, even without any compiler optimizations that eliminate the check. So this is simply replacing a potential for undefined behavior that is impossible (provable by looking only at this source file) but gcc (and not clang) falsely warn about, with a potential for new undefined behavior that gcc doesn't warn about even though it cannot be ruled out, at least not without analyzing other source files. As an example, suppose it was possible for some other source file to set track->entry to INT_MIN. Previously the comparison would be true and it would safely return 0, but with the proposed change it would result in undefined behavior and may attempt to access entries in the cluster array that do not exist. This is different than quieting a warning by unnecessarily initializing a local variable to 0, because changing a local variable from uninitialized to initialized does not introduce any new undefined behavior. - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
