Remove NULL-checks before free()

[mmcco]

free() has been specified as NULL-safe since C89. All modern systems and almost all prehistoric ones have NULL-safe free implementations. Removing this dead logic often makes code significantly more readable.

[lichray]

I plan to accept this type of change, but please take a look at https://github.com/lichray/nvi2/pull/19/files as well, which has better spacing and covers some more places.

[mmcco]

Why wait? I'm fine with either, and can do a second pass afterward.

[lichray]

Let's work on this. Please review 9f7600afdddbb460a1c5c282ded0bdd82a5f50a6

[mmcco]

Is that both of the diffs merged?

The last hunk in cl/cl_screen.c looks mistaken because clp->cw.blen1 = 0; is pulled out of the condition. This may be logically fine, but it's a behavioral change and could introduce a bug. I'm unfamiliar with the code and therefore have no idea.

Anecdotally, this turns text_free() into an alias for free(). Unless text_free() part of an API with third-party code, we can probably just replace it in a later commit.

There are a few instances right above the hunk in regex/regfree.c that were obfuscated by needless argument casts. However, I think we should commit a subset of the current diff and leave the rest for a second pass.

There are instances whose arguments are passed to the macros O_STR(), O_D_STR(), and _HMAP(). We have to be careful with this type because we could overlook a state change contained in a macro. I looked at these macros' definitions and none seem to change state. However, the implementations of *_STR() are dismally complicated. We should really start converting macro logic to functions were possible, but that's a separate issue.

So, barring the struct field assignment I mentioned above, this looks good to me.

[lichray]

On Mon, Dec 28, 2015 at 10:05 PM, Michael McConville < notifications@github.com> wrote:

Is that both of the diffs merged?

Yes, with 1 case unchanged here, but changed in Anthony's patch.

The last hunk in cl/cl_screen.c looks mistaken because clp->cw.blen1 = 0; is pulled out of the condition. They may be logically fine, but it's a behavioral change and could introduce a bug. I'm unfamiliar with the code and therefore have no idea.

It's cleanup code, no code is relying on blen1, so it's fine.

Anecdotally, this turns text_free() into an alias for free(). Unless it's part of an API with third-party code, we can probably just replace it in a later commit.

It's 2 frees, and it's used in other files.

There are a few instances right above the hunk in regex/regfree.c that were obfuscated by needless argument casts. However, I think we should commit a subset of the current diff and leave the rest for a second pass.

OK.

There are instances whose arguments are passed to the macros O_STR, O_D_STR, and _HMAP. We have to be careful with this type because we could overlook a state change contained in a macro. I looked at these macros' definitions and none seem to change state. However, the implementations of *_STR are dismally complicated. We should really start converting macro logic to functions were possible, but that's a separate issue.

I guess this won't help. This is C not C++, C function cannot return lvalue (for assignment)...

So, barring the struct field assignment I mentioned above, this looks good to me.

Thanks.

Zhihao Yuan, ID lichray The best way to predict the future is to invent it.

---

4BSD -- http://bit.ly/blog4bsd

[mmcco]

Anecdotally, this turns text_free() into an alias for free(). Unless it's part of an API with third-party code, we can probably just replace it in a later commit.

It's 2 frees, and it's used in other files.

Oh right. Good catch.

In terms of the macros: I just meant that they should be translated to functions in the general sense. Using macro logic that complex is asking for trouble.

However, in response to this:

I guess this won't help. This is C not C++, C function cannot return lvalue (for assignment)...

Macros used like that could totally change state. Consider something as simple as:

#define MY_MACRO(p)        (p++, p->fld)