On Thu, 21 Apr 2022 17:41:36 +0200, Ævar Arnfjörð Bjarmason wrote: > First, I wondered if we were missing _(), but looking at other string in > the file they're not using that already, looks like these all should, > but we can fix this all up some other time in some i18n commit. It's > fine to keep this for now. Yes, I agree with you. I also think I have a willingness to make another patchset to solve the _() missing problems recently. > But more importantly: I think this should be your 4/5. I.e. just make > these an error() and you won't need to add e.g. this > trace2_data_string() for a failed stat. Make sense. Will adjust the order in next path. > You will be inside your trace2 region, so any failure to stat etc. will > be obvious from the structure of the data and the "error" event, no > reason to have an additional trace2_data_string(). Yeah, I forgot about the "error()" already load the trace2 functions in. Will remove the redundant trace2_data_string() where it's obviously will return error(). > Aside from that & as a general matter: Unless you have some use-case for > trace2 data in this detail I'd think that it would be better just to > skip logging it (except if we get it for free, such as with error()). > > I.e. is this particular callsite really different from other places we > fail to stat() or open() something? > > It's all a moot point with the region + error, but just something to > keep in mind. Make sense. And I think it's a good case in "open_midx_bitmap_1()" to add related trace2_data_string() because there only a general error info in "cleanup:". Thanks.
