On Mon, Mar 18, 2024 at 2:28 AM Peter Eisentraut <pe...@eisentraut.org> wrote:
> On 21.02.24 08:26, Peter Eisentraut wrote: > > On 14.02.24 17:52, Peter Eisentraut wrote: > >> A gentler way might be to start using some perlcritic policies like > >> InputOutput::RequireCheckedOpen or the more general > >> InputOutput::RequireCheckedSyscalls and add explicit error checking at > >> the sites it points out. > > > > Here is a start for that. I added the required stanza to perlcriticrc > > and started with an explicit list of functions to check: > > > > functions = chmod flock open read rename seek symlink system > > > > and fixed all the issues it pointed out. > > > > I picked those functions because most existing code already checked > > those, so the omissions are probably unintended, or in some cases also > > because I thought it would be important for test correctness (e.g., some > > tests using chmod). > > > > I didn't design any beautiful error messages, mostly just used "or die > > $!", which mostly matches existing code, and also this is > > developer-level code, so having the system error plus source code > > reference should be ok. > > > > In the second patch, I changed the perlcriticrc stanza to use an > > exclusion list instead of an explicit inclusion list. That way, you can > > see what we are currently *not* checking. I'm undecided which way > > around is better, and exactly what functions we should be checking. (Of > > course, in principle, all of them, but since this is test and build > > support code, not production code, there are probably some reasonable > > compromises to be made.) > > After some pondering, I figured the exclude list is better. So here is > a squashed patch, also with a complete commit message. > > Btw., do we check perlcritic in an automated way, like on the buildfarm? Yes. crake and koel do. cheers andrew
