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?