Fix up pointer issues with lib/libkvm

[ngie]

Summary

Fix up pointer issues with lib/libkvm

In particular,

• avoid dereferencing NULL pointers
• test pointers against NULL, not 0
• test for errout == NULL in the top-level functions (kvm_open, kvm_openfiles, kvm_open2, etc)

Found with: devel/cocchinelle

MFC after: 1 week
Sponsored by: EMC / Isilon Storage Division

[ngie]

lib/libkvm/kvm.c

487–490

I should add a note about this.. in particular, _kvm_open handles the errout == NULL case, but none of the top-level functions did in this case

[ngie]

lib/libkvm/kvm.c

382–385

These should be NULL.

lib/libkvm/kvm_proc.c

640

Oy. Missed this case

642

This should be NULL, not 0.

649

Should be reallocf.

651

This should be NULL... now it makes sense why this wasn't working with coccinelle...

[ngie]

• Fix some accidentally overlooked items
• Use reallocf instead of realloc[+free on failure]

[araujo]

Hi,

I have two dummy questions:

1. Is it really necessary those #if #else?
2. At line 640 would not hurt keep that if(kd->procbase) before call free. NOTE: I didn't make any test with the code, I don't really know if always when call _kvm_freeprocs() the pointer will have data.

Best,

[jhb]

lib/libkvm/kvm.c

487–490

I've never considered errout an optional argument here. The manpage for kvm_open() has more details, but kvm_open() is a legacy API for compatiblity with ancient SunOS and uses errstr == NULL to mean that errors should not be printed to stderr by the library directly.

The more recent kvm_openfiles() and kvm_open2() do not ever print errors from the library to stderr directly. Instead, callers use kvm_geterror() to access error messages. However, 'errstr' must point to a valid buffer of the desired size to handle errors during open since kvm_geterror() requires an open kvm handle.

OTOH, it seems that the inconsistency in _kvm_open() and kvm_openfiles() goes back to FreeBSD 1.1 at least.

Looking in CSRG history, it seems that this goes back to some commit by Kirk of a patch from someone else to add the kvm_open() compat layer to the BSD libkvm. Prior to that there was just a global errbuf variable that kvm_geterr() returned.

lib/libkvm/kvm_file.c

74 ↗

(On Diff #15210)

I think kvm_file.c should just be removed in 11 instead (as a separate commit). I wouldn't bother touching it in this one.

lib/libkvm/kvm_proc.c

650–651

Since you've uninlined reallocf() here, you could in theory just reuse p and not bother with 'np' at all:

p = reallocf(p, n);

However, I don't think it really matters either way.

[jhb]

In D5954#126964, @araujo wrote:

Hi,

I have two dummy questions:

1. Is it really necessary those #if #else?
2. At line 640 would not hurt keep that if(kd->procbase) before call free. NOTE: I didn't make any test with the code, I don't really know if always when call _kvm_freeprocs() the pointer will have data.

Best,

For 1), I vote to kill kvm_file.c and kvm_getfile() in 11.

For 2), free() is defined to be a no-op if you pass in NULL, so it's ok to remove the if.

[ngie]

lib/libkvm/kvm_file.c

74 ↗

(On Diff #15210)

kvm_getfiles needs to be removed then as it's a public API. Should I do that?

[ngie]

lib/libkvm/kvm.c

487–490

I understand the concern, but unfortunately kvm(3) doesn't enforce that requirement (it's assumed).

Should errout be tested for != NULL and return EINVAL?

[jhb]

lib/libkvm/kvm.c

487–490

Your approach is probably the most flexible to allow the error reporting to be optional, so you can leave it as is.

lib/libkvm/kvm_file.c

74 ↗

(On Diff #15210)

Yes. It hasn't worked in a long time. We've already bumped libkvm's major version to add kvm_open2() and friends for cross-debugging support, so it's actually not a bad time to do this.

[ngie]

Fix some more missing 0 -> NULL conversions in kvm_args

[ngie]

Remove unrelated kvm.h diff