[PATCH kconfig] Add annotation symbol to configuration handling.

[PATCH kconfig] Add annotation symbol to configuration handling.

[Christian Melki]

While maintaining various projects and kernels, I've often found myself
thinking "Why is this selected / configured?".

Sometimes I'll find information in a disjunct documentation system or in
the head of somebody else. But most of the time that piece of
information is just lost. Configurations get moved from various
repositories, so that type of disconnected information also gets trashed.

It would be nice if the configuration supported some form of simple
annotation to variable mechanism. Ie, part of the actual config
(only during read / write) and not just a whashed-away comment.

$ grep ANNOTATE_ .config
ANNOTATE_CONFIG_TRANSPARENT_HUGEPAGE_MADVISE="Always was causing issues."
ANNOTATE_CONFIG_HID_SENSOR_HUB="Plus IIO for the Realsense camera support."
ANNOTATE_CONFIG_HID_SENSOR_ACCEL_3D="Used by Intel Realsense camera."
ANNOTATE_CONFIG_HID_SENSOR_GYRO_3D="Used by Intel Realsense camera."

Signed-off-by: Christian Melki <christian.melki@t2data.com>
---
 scripts/kconfig/confdata.c | 38 ++++++++++++++++++++++++++++++++++++++
 scripts/kconfig/expr.h     |  9 +++++++++
 scripts/kconfig/lkc.h      | 13 ++++++++++++-
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 2568dbe16ed6..8fb198f297d7 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -167,6 +167,7 @@ static int conf_touch_dep(const char *name)
 struct conf_printer {
 	void (*print_symbol)(FILE *, struct symbol *, const char *, void *);
 	void (*print_comment)(FILE *, const char *, void *);
+	void (*print_annotation)(FILE *, struct symbol *, const char *);
 };
 
 static void conf_warning(const char *fmt, ...)
@@ -478,6 +479,30 @@ int conf_read_simple(const char *name, int def)
 			}
 			if (conf_set_sym_val(sym, def, def_flags, p))
 				continue;
+		} else if (!memcmp(line, ANNOTATE_, strlen(ANNOTATE_))) {
+			p = strchr(line + strlen(ANNOTATE_) + strlen(CONFIG_), '=');
+			if (!p)
+				continue;
+			*p++ = 0;
+			p2 = strchr(p, '\n');
+			if (p2) {
+				*p2-- = 0;
+				if (*p2 == '\r')
+					*p2 = 0;
+			}
+			sym = sym_find(line + strlen(ANNOTATE_));
+			if (!sym)
+				continue;
+			if (*p++ != '"')
+				continue;
+			for (p2 = p; (p2 = strpbrk(p2, "\"\\")); p2++) {
+				if (*p2 == '"') {
+					*p2 = 0;
+					break;
+				}
+				memmove(p2, p2 + 1, strlen(p2));
+			}
+			sym->annotation.val = xstrdup(p);
 		} else {
 			if (line[0] != '\r' && line[0] != '\n')
 				conf_warning("unexpected data: %.*s",
@@ -631,10 +656,17 @@ kconfig_print_comment(FILE *fp, const char *value, void *arg)
 	}
 }
 
+static void
+kconfig_print_annotation(FILE *fp, struct symbol *sym, const char *value)
+{
+	fprintf(fp, "%s%s=%s\n", ANNOTATE_, sym->name, value);
+}
+
 static struct conf_printer kconfig_printer_cb =
 {
 	.print_symbol = kconfig_print_symbol,
 	.print_comment = kconfig_print_comment,
+	.print_annotation = kconfig_print_annotation,
 };
 
 /*
@@ -729,6 +761,12 @@ static void conf_write_symbol(FILE *fp, struct symbol *sym,
 		str = sym_get_string_value(sym);
 		printer->print_symbol(fp, sym, str, printer_arg);
 	}
+
+	if (sym->annotation.val) {
+		str = sym_escape_string_value(sym->annotation.val);
+		printer->print_annotation(fp, sym, str);
+		free((void *)str);
+	}
 }
 
 static void
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 5c3443692f34..29e1419b51ef 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -74,6 +74,10 @@ enum {
 	S_DEF_COUNT
 };
 
+struct symbol_annotation {
+	char *val;
+};
+
 /*
  * Represents a configuration symbol.
  *
@@ -103,6 +107,11 @@ struct symbol {
 	 */
 	struct symbol_value def[S_DEF_COUNT];
 
+	/*
+	 * Annotation string for symbol.
+	 */
+	struct symbol_annotation annotation;
+
 	/*
 	 * An upper bound on the tristate value the user can set for the symbol
 	 * if it is a boolean or tristate. Calculated from prompt dependencies,
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index bee2413bda63..8caf2fd04be5 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -27,12 +27,23 @@ extern "C" {
 #ifndef CONFIG_
 #define CONFIG_ "CONFIG_"
 #endif
+#ifndef ANNOTATE_
+#define ANNOTATE_ "ANNOTATE_CONFIG_"
+#endif
+
 static inline const char *CONFIG_prefix(void)
 {
-	return getenv( "CONFIG_" ) ?: CONFIG_;
+	return getenv("CONFIG_") ?: CONFIG_;
 }
 #undef CONFIG_
+
 #define CONFIG_ CONFIG_prefix()
+static inline const char *ANNOTATE_prefix(void)
+{
+	return getenv("ANNOTATE_") ?: ANNOTATE_;
+}
+#undef ANNOTATE_
+#define ANNOTATE_ ANNOTATE_prefix()
 
 enum conf_def_mode {
 	def_default,
-- 
2.31.1

Re: [PATCH kconfig] Add annotation symbol to configuration handling.

[Boris Kolpackov]

Christian Melki <christian.melki@t2data.com> writes:

> While maintaining various projects and kernels, I've often found myself
> thinking "Why is this selected / configured?".
> 
> Sometimes I'll find information in a disjunct documentation system or in
> the head of somebody else. But most of the time that piece of
> information is just lost. Configurations get moved from various
> repositories, so that type of disconnected information also gets trashed.
> 
> It would be nice if the configuration supported some form of simple
> annotation to variable mechanism. Ie, part of the actual config
> (only during read / write) and not just a whashed-away comment.
> 
> $ grep ANNOTATE_ .config
> ANNOTATE_CONFIG_TRANSPARENT_HUGEPAGE_MADVISE="Always was causing issues."
> ANNOTATE_CONFIG_HID_SENSOR_HUB="Plus IIO for the Realsense camera support."
> ANNOTATE_CONFIG_HID_SENSOR_ACCEL_3D="Used by Intel Realsense camera."
> ANNOTATE_CONFIG_HID_SENSOR_GYRO_3D="Used by Intel Realsense camera."

Just to confirm my understanding, these annotations are expected to be
added manually, correct? The proposed patch only makes sure they are
preserved during the configuration read-write cycle.

Re: [PATCH kconfig] Add annotation symbol to configuration handling.

[Christian Melki]

On 5/11/21 3:31 PM, Boris Kolpackov wrote:
> Christian Melki <christian.melki@t2data.com> writes:
> 
>> While maintaining various projects and kernels, I've often found myself
>> thinking "Why is this selected / configured?".
>>
>> Sometimes I'll find information in a disjunct documentation system or in
>> the head of somebody else. But most of the time that piece of
>> information is just lost. Configurations get moved from various
>> repositories, so that type of disconnected information also gets trashed.
>>
>> It would be nice if the configuration supported some form of simple
>> annotation to variable mechanism. Ie, part of the actual config
>> (only during read / write) and not just a whashed-away comment.
>>
>> $ grep ANNOTATE_ .config
>> ANNOTATE_CONFIG_TRANSPARENT_HUGEPAGE_MADVISE="Always was causing issues."
>> ANNOTATE_CONFIG_HID_SENSOR_HUB="Plus IIO for the Realsense camera support."
>> ANNOTATE_CONFIG_HID_SENSOR_ACCEL_3D="Used by Intel Realsense camera."
>> ANNOTATE_CONFIG_HID_SENSOR_GYRO_3D="Used by Intel Realsense camera."
> 
> Just to confirm my understanding, these annotations are expected to be
> added manually, correct? The proposed patch only makes sure they are
> preserved during the configuration read-write cycle.
> 

Correct. The example does not do away with annotations if the symbol is
deselected, but that can be changed. I'd prefer if annotations could
live a life as long as the symbol exists, that way you can follow
deselected symbols (what they were used for, if one cares to add
information).