[Lldb-commits] [PATCH] D12994: Improve support of the ncurses dependency on NetBSD

[Bruce Mitchener via lldb-commits]

brucem added a subscriber: brucem.
brucem added a comment.

Some mostly superficial comments.

I'd like to hear if anyone else has an opinion about using a Config.h generated by cmake & autotools.

I'm also not sure offhand how this would impact the Xcode build that the Apple folks use.

And I didn't see anything that sets autoconf stuff to generate that new config.h.in?


================
Comment at: cmake/modules/LLDBConfig.cmake:284
@@ +283,3 @@
+
+    find_library(NCURSES_PANEL_LIBRARY NAMES panel DOC "The ncureses panel library")
+    if (CURSES_FOUND)
----------------
Typo here: `ncureses`

================
Comment at: include/lldb/Config/config.h.cmake:6
@@ +5,3 @@
+#else
+#define CONFIG_H
+
----------------
This should be `LLDB_CONFIG_CONFIG_H` rather than just `CONFIG_H`.

Also, why not use the same `#ifndef` technique that everything else uses in the header files?

================
Comment at: include/lldb/Config/config.h.cmake:6
@@ +5,3 @@
+#else
+#define CONFIG_H
+
----------------
brucem wrote:
> This should be `LLDB_CONFIG_CONFIG_H` rather than just `CONFIG_H`.
> 
> Also, why not use the same `#ifndef` technique that everything else uses in the header files?
Also, other headers in LLDB tend to have capitalized names, so maybe this should be `include/lldb/Config/Config.h.cmake`.

================
Comment at: include/lldb/Config/config.h.in:6
@@ +5,3 @@
+#else
+#define CONFIG_H
+
----------------
Same as for `config.h.cmake`.


Repository:
  rL LLVM

http://reviews.llvm.org/D12994