On Tue, Jun 18, 2019 at 09:13:19AM -0700, Shawn Debnath wrote: >> case SLRU_WRITE_FAILED: >> ereport(ERROR, >> (errcode_for_file_access(), >> errmsg("could not access status of >> transaction %u", xid), >> - errdetail("Could not write to file >> \"%s\" at offset %u: %m.", >> - path, offset))); >> + errno == 0 >> + ? errdetail("Short write to file >> \"%s\" at offset %u.", path, offset) >> + : errdetail("Could not write to file >> \"%s\" at offset %u: %m.", >> + path, >> offset)));
There is no point to call errcode_for_file_access() if we know that errno is 0. Not a big deal, still.. The last time I looked at that, the best way I could think of would be to replace slru_errcause with a proper error string generated at error time. Perhaps the partial read/write case does not justify this extra facility. I don't know. At least that would be more flexible. > Similarly, "Encountered partial write to file ..." would be better? Not > a 100% on using "Encountered" but "partial" seems to be the right word > to use here. 811b6e36 has done some work so as we have more consistency in the error messages and I don't think we should introduce more flavors of this stuff as that makes the life of translators harder. - if (CloseTransientFile(fd)) + if (CloseTransientFile(fd) < 0) Some spots are missing: $ git grep "CloseTransientFile" | grep "))" | wc -l 30 -- Michael
signature.asc
Description: PGP signature