On Thu Oct 12 04:24:03 2023 +0000, Zebediah Figura wrote:
My (re)interpretation of error levels has been that ERR should be for internal errors that a well-behaved application doesn't trigger, whereas WARN is for errors that might happen naturally but might also be symptomatic of a failure. Wine code doesn't always follow this interpretation (although I think most of the more well-maintained subsystems do), but I think it broadly makes sense given what's printed by default. (And usually where Wine code doesn't follow this, it errs on the side of being *more* verbose rather than less.) In this case the motivation for the change is that almost none of those calls should fail if we're passing valid parameters (and not fatally out of memory). If they do fail, it's an internal error and we should be noisy about it; I'd think an assert would not be unwarranted even. This is also the motivation for changing the control flow, because it's really just not worth the extra code to handle the failure. The original motivation for this is that the if/else model doesn't work well if we need to do something nontrivial between two method calls. As you say, though, a set-data-format helper would also achieve this, so I can revert this patch and do that instead; I don't feel that strongly about it. (I do still feel that ERR is more appropriate than WARN, though.)
Okay, I'm fine with changing WARN to ERR with this rationale, though it should be a separate change.