strdup() uses M_WAITOK inside. It can not fail.

Ah, right you are. Good catch.

nit: sys/types is redundant here with sys/param at the top

I think we should probably log a reason when we fail attach (and there isn't concern we're the wrong driver for the hardware).

Also, stylistically, I'd suggest a single exit path that frees these allocations.

// much earlier:
maps = NULL;
sb = NULL;

if (num_maps == 0) {
  error = ENXIO;
  log();
  goto out;
}

Should we do this per-map instead of OR'ing all maps' flags together?

For sbuf it is canonical to just do error detection at finish(), as long as there isn't anything super expensive between the first possible failure and finish. It would be a good fit here, IMO.

We really should have a sbuf_detach() function for this but we don't appear to. strdup works.

e.g.,

  if (error == 0) {
    ...; // as-is
  }
  error = 0; // missing label is ok
out:
  if (error != 0) {
    free(nv->nv_flush_addr, ...);
    free(maps, ...);
  }
  return (error);

So it is! And in fact, style(9) mentions that. I'll remove it.

(<rant-let>I much prefer the way we do it at ${JOB}, where everything includes what it uses, without magic undeclared dependencies.</rant-let>)

Also, stylistically, I'd suggest a single exit path that frees these allocations.

That's also my preference, when left to my own devices. (And my own device drivers; see jedec_dimm(4) and imcsmb(4). 😜)

But that's not the style used in this driver, and I generally try to match the rest of the file.

As explained in the comment, all these maps are associated with the same NVDIMM, so a failure on any one of the maps needs to be reported for the whole device.

(FWIW, neither I nor @mav have seen more than a single map on a given device, though a single map might span multiple devices.)

Noted.

Ooo, you're right about that one too!