From sgrubb at redhat.com  Fri Jul 21 19:53:12 2017
From: sgrubb at redhat.com (Steve Grubb)
Date: Fri, 21 Jul 2017 13:53:12 -0400
Subject: [Rd] [PATCH] Fix missing break
In-Reply-To: <22897.47489.517803.232885@stat.math.ethz.ch>
References: <60035856.27PnXlZqtm@x2> <4241711.WyxJnDo7EK@x2>
 <22897.47489.517803.232885@stat.math.ethz.ch>
Message-ID: <2491471.UUKbAB2nFm@x2>

Hello Martin,

On Friday, July 21, 2017 4:21:21 AM EDT Martin Maechler wrote:
> I have now created an account for you.

Thanks. Is that the preferred method of transferring these patches?


> >> In examples like the one below, if you have R code that shows symptoms,
> >> it would really help in the bug report.
> > 
> > I am hoping that we can look at the code as seasoned programmers and say
> > yeah, that is a bug.
> 
> I agree in this case.
> OTOH, it is exactly one of the case where the bug is not
> triggerable currently:
> 
>   al <- formals(ls); length(al) <- 3
> 
> would trigger the bug... but you get an error message ".. vector .."
> and as I now found that is from a slightly misguided check:
> isVectorizable()  is not approriate here and should really be
> replaced by isList().
> 
> So .. indeed, your report will have triggered an improvement in
> the code, which I'm about to commit.

That's what it's all about.  :-)
 
> Thank you very much Steve!
> 
> > I run the code through Coverity and have quite a lot of
> > problems to tell you about.
> 
> I'm not the expert on static code analysis, but as a seasoned
> statistician (*and* from experience with other such analyses) I
> know that you always get false positives.

Absolutely. I weeded the report down to 15 issues to start with. There are 
also ways to annotate the code so that checkers dismiss something it would 
otherwise be inclined to report.

> >> Otherwise, someone else will have to analyze the code to decide whether
> >> it's a bug or missing comment.  That takes time, and if there are no
> >> known symptoms, it's likely to be assigned a low priority.  The sad
> >> truth is that very few members of R Core are currently actively fixing
> >> bugs.
> > 
> > That's a shame. I'd be happy to give the scan to people in core so they
> > can see what the lay of the land looks like.
> 
>  (hmm... the above does look a teeny tiny bit arrogant in my
>   eyes; but then I'm not a native English (nor "American" 
>   speaker ...)

I apologize if that is the way it came across. "That's a shame" can also mean 
"That's unfortunate" because I was thinking that I spent some time fixing up 
patches that might not be wanted. However, I see that you have looked at the 
patches and I thank you for that.  :-)

The second sentence above is an honest offer. I'd be happy to send the output 
of the report off list (in case anything sensitive is listed). In this and the 
other patches I haven't sent, I'm just picking the low hanging fruit.

-Steve



[Rd] [PATCH] Fix missing break
Steve Grubb