Copilot commented on code in PR #20628:
URL: https://github.com/apache/nuttx/pull/20628#discussion_r4257781048
##########
libs/libc/libc.h:
##########
@@ -333,7 +333,11 @@ int lib_restoredir(void);
/* Defined in lib_cxx_initialize.c */
+#ifdef CONFIG_HAVE_CXXINITIALIZE
void lib_cxx_initialize(void);
+#else
+# define lib_cxx_initialize()
Review Comment:
Using a function-like macro as the fallback can cause surprising
preprocessor substitution if `lib_cxx_initialize` is ever referenced as an
identifier (e.g., passed as a callback/function pointer, used in logs with
`%p`, etc.). A safer, type-checked alternative is a `static inline void
lib_cxx_initialize(void) {}` stub under `#else`, which still optimizes away in
normal call sites while preserving C semantics and reducing macro-related
pitfalls.
##########
mm/mm_heap/mm_addfreechunk.c:
##########
@@ -0,0 +1,41 @@
+/****************************************************************************
+ * mm/mm_heap/mm_addfreechunk.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/mm/mm.h>
+
+/* Emit the out-of-line definition of mm_addfreechunk() from mm.h. */
+#define MM_ADDFREECHUNK_IMPL
+#include "mm_heap/mm.h"
+
Review Comment:
The out-of-line instantiation relies on `MM_ADDFREECHUNK_IMPL` being defined
before the first inclusion of `mm_heap/mm.h`. Including `<nuttx/mm/mm.h>`
before defining the macro is risky if it (directly or indirectly) includes
`mm_heap/mm.h` first; that would trip the include guard and prevent the
definition from being emitted, leading to a missing symbol at link time when
`CONFIG_MM_INLINE_ADDFREECHUNK` is disabled. Fix by ensuring
`MM_ADDFREECHUNK_IMPL` is defined before any header that might include
`mm_heap/mm.h` (e.g., move the define + `#include \"mm_heap/mm.h\"` above
`<nuttx/mm/mm.h>`, or drop `<nuttx/mm/mm.h>` from this TU if it’s not required).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
