Upstream Commit:

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/commit/afb0e2c53c4c17dfdb89d63b39db5101cc864704

SUMMARY:

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/commit/afb0e2c53c4c17dfdb89d63b39db5101cc864704 is 8 years old. This is present in all RHEL versions already (and in the meantime the code likely changed again).

In any case, the patch is so old, there is little reason to suspect a bug in NetworkManager of the last years.

Also, this patch was not to fix any actual issue (because the patch doesn't actually change visibly the behavior). It was done to silence a coverity false-positive warning. 8 years ago, a coverity warning was investigated. It was concluded that it's a false positive, and the patch done to avoid the warning. As such, it's not CVE worthy. I'd wish such bogus CVEs would not be reported. They only cause work :) 

---

```
     item = priority_map_new_from_str (map, str);
-    g_return_val_if_fail (item != NULL, FALSE);
+    if (!item)
+        g_return_val_if_reached (FALSE);
```

glib's g_return*() macros emit a `g_critical()` message. These are assertions, as also documented in our CONTRIBUTING file ([1]).

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/3fd28a11d4bb9e7cfd9f86bc3af90a71a447f7fc/CONTRIBUTING.md#assertions-in-networkmanager-code

There really is not much point in reasoning about what happens after an assertion fails. When you ever encounter an assertion failure, then the bug happened before that point already, and the fix must be before that point (unless the assertion statement itself is wrong).


Btw, it must also always be possible to build NetworkManager with G_DISABLE_CHECKS, G_DISABLE_ASSERT, NDEBUG to strip out all assertions. If you encounter a bug in such a build configuration, it would also be a bug with assertions enabled. Granted, all our NetworkManager builds in RHEL/Fedora do NOT strip out such asserts.


Also, unless you build with G_DISABLE_CHECKS (which we don't), then above patch (replacing `g_return_val_if_fail()` with `if (!cond) g_return_if_reached()` is identical. Also for that reason this patch cannot fix any bug.




Sometimes we might make a lukewarm effort to salvage the program after an assertion fails. In particular, `g_return*()` already affects the code flow and tries to just error out (it already make an attempt to maybe be able to proceed afterwards). It's not possible because an assertion failure is an unexpected event that shouldn't happen, we don't know what caused it or how to recover from it.


For example, a common glib pattern (heavily used in NetworkManager) is that functions which fail can retrun a GError output:
```
NMRange *
nm_range_from_str(const char *str, GError **error)
{
    gs_free char *str_free = NULL;
    guint64       start;
    guint64       end = 0;
    char         *c;

    g_return_val_if_fail(str, NULL);
    g_return_val_if_fail(!error || !*error, NULL);

    ...
}
```
For such functions, an GError MUST be returned if-and-only-if the function fails. A common pattern for the caller would be:
```
    range = nm_range_from_str(str, &error);
    if (!range) {
        log("error happened: %s", error->str);
        return NULL;
    }
```
Note that if the code fails the `g_return_val_if_fail()` path, the output error is not set, and the caller will crash right after. We don't try to work around that further by doing:
```
-    g_return_val_if_fail(str, NULL);
+    if (!str) {
+         g_set_error(error, ...);
+         g_return_val_if_reached(NULL);
+    }
```
and we also don't expect the caller do do:
```
    range = nm_range_from_str(str, &error);
    if (!range) {
        log("error happened: %s", error ? error->str : "unknown error");
        return NULL;
    }
```
Instead, the caller MUST use nm_range_from_str() according to the API (that is, provide a valid str argument) and the caller can rely that the function behaves according to the API (return a GError on failure).

The solution is of course, to ensure that `nm_range_from_str()` is called with a valid string.

---

Coverity sometimes doesn't get this (especially 8 years ago, which is huge in terms of how coverity improved). So it can make sense to make an effort to pacify coverity. Which is what the patch https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/commit/afb0e2c53c4c17dfdb89d63b39db5101cc864704 does. Nothing more.