Patchwork [2/2] Fix build without CONFIG_NO_CONFIG_WRITE enabled.

login
register
mail settings
Submitter Solomon Peachy
Date March 11, 2013, 9:34 p.m.
Message ID <1363037698-22826-3-git-send-email-pizza@shaftnet.org>
Download mbox | patch
Permalink /patch/226708/
State Changes Requested
Headers show

Comments

Solomon Peachy - March 11, 2013, 9:34 p.m.
Signed-off-by: Solomon Peachy <pizza@shaftnet.org>
---
 wpa_supplicant/config.c    | 10 +++++-----
 wpa_supplicant/wpas_glue.c |  6 ++++++
 2 files changed, 11 insertions(+), 5 deletions(-)
Jouni Malinen - March 16, 2013, 10:32 a.m.
On Mon, Mar 11, 2013 at 05:34:58PM -0400, Solomon Peachy wrote:
> +#ifndef NO_CONFIG_WRITE
>  static char * wpa_config_write_eap(const struct parse_data *data,
>  				   struct wpa_ssid *ssid)

NO_CONFIG_WRITE is not defined anywhere.. Was this supposed to be
CONFIG_NO_CONFIG_WRITE?

> +#ifndef NO_CONFIG_WRITE
>  /**
>   * wpa_config_get_all - Get all options from network configuration

Though, this would break a build that has CONFIG_NO_CONFIG_WRITE=y and
D-Bus interface enabled..

> diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
> +#ifndef CONFIG_NO_CONFIG_WRITE
>  	if (wpa_s->conf->update_config) {
>  		int ret = wpa_config_write(wpa_s->confname, wpa_s->conf);

While this can be used to reduce the binary size a bit, this is not
needed to fix the build as far as I can tell.

> @@ -798,8 +802,10 @@ int wpa_supplicant_init_eapol(struct wpa_supplicant *wpa_s)
> +#ifndef CONFIG_NO_CONFIG_BLOBS
>  	ctx->set_config_blob = wpa_supplicant_set_config_blob;
>  	ctx->get_config_blob = wpa_supplicant_get_config_blob;
> +#endif /* CONFIG_NO_CONFIG_BLOBS */

This is a valid fix for CONFIG_NO_CONFIG_BLOBS, but that does not match
with the commit log for this nor is this a complete solution (D-Bus code
was similar cases). I fixed the CONFIG_NO_CONFIG_BLOBS part in a
different commit that addresses all cases.

I did not apply the CONFIG_NO_CONFIG_WRITE parts because I cannot
reproduce build failure and the use of NO_CONFIG_BLOBS (vs.
CONFIG_NO_CONFIG_BLOBS) did not look correct.
Solomon Peachy - March 16, 2013, 12:30 p.m.
On Sat, Mar 16, 2013 at 12:32:35PM +0200, Jouni Malinen wrote:
> On Mon, Mar 11, 2013 at 05:34:58PM -0400, Solomon Peachy wrote:
> > +#ifndef NO_CONFIG_WRITE
> >  static char * wpa_config_write_eap(const struct parse_data *data,
> >  				   struct wpa_ssid *ssid)
> 
> NO_CONFIG_WRITE is not defined anywhere.. Was this supposed to be
> CONFIG_NO_CONFIG_WRITE?

Actually, it is correct -- if you look at the top of config.c, you'll 
see this:

#if !defined(CONFIG_CTRL_IFACE) && defined(CONFIG_NO_CONFIG_WRITE)
#define NO_CONFIG_WRITE
#endif

It's kinda silly, IMO, but I was going for minimal patches to solve the 
build problems I had.

> 
> > +#ifndef NO_CONFIG_WRITE
> >  /**
> >   * wpa_config_get_all - Get all options from network configuration
> 
> Though, this would break a build that has CONFIG_NO_CONFIG_WRITE=y and
> D-Bus interface enabled..

I suppose the most expedient manner here is to add CONFIG_DBUS to the 
conditions that prevents NO_CONFIG_WRITE from being defined.

> > diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c 
> > +#ifndef CONFIG_NO_CONFIG_WRITE
> >  	if (wpa_s->conf->update_config) {
> >  		int ret = wpa_config_write(wpa_s->confname, wpa_s->conf);
> 
> While this can be used to reduce the binary size a bit, this is not
> needed to fix the build as far as I can tell.

Upon further digging, you're correct -- All of the bundled config_*.c 
implementations wrap the guts of wpa_config_write with 
CONFIG_NO_CONFIG_WRITE.  I could have written a dummy wpa_config_write() 
function instead, I suppose.
 
> > @@ -798,8 +802,10 @@ int wpa_supplicant_init_eapol(struct wpa_supplicant *wpa_s)
> > +#ifndef CONFIG_NO_CONFIG_BLOBS
> >  	ctx->set_config_blob = wpa_supplicant_set_config_blob;
> >  	ctx->get_config_blob = wpa_supplicant_get_config_blob;
> > +#endif /* CONFIG_NO_CONFIG_BLOBS */
> 
> This is a valid fix for CONFIG_NO_CONFIG_BLOBS, but that does not match
> with the commit log for this nor is this a complete solution (D-Bus code
> was similar cases). I fixed the CONFIG_NO_CONFIG_BLOBS part in a
> different commit that addresses all cases.

Great, thanks.

> I did not apply the CONFIG_NO_CONFIG_WRITE parts because I cannot
> reproduce build failure and the use of NO_CONFIG_BLOBS (vs.
> CONFIG_NO_CONFIG_BLOBS) did not look correct.

I think I've explained that now.  (and you meant *_WRITE, not *_BLOBS, correct?)

 - Solomon

Patch

diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
index 6860765..cec2516 100644
--- a/wpa_supplicant/config.c
+++ b/wpa_supplicant/config.c
@@ -1021,6 +1021,7 @@  static int wpa_config_parse_eap(const struct parse_data *data,
 }
 
 
+#ifndef NO_CONFIG_WRITE
 static char * wpa_config_write_eap(const struct parse_data *data,
 				   struct wpa_ssid *ssid)
 {
@@ -1054,7 +1055,7 @@  static char * wpa_config_write_eap(const struct parse_data *data,
 
 	return buf;
 }
-
+#endif /* NO_CONFIG_WRITE */
 
 static int wpa_config_parse_password(const struct parse_data *data,
 				     struct wpa_ssid *ssid, int line,
@@ -1135,7 +1136,7 @@  static int wpa_config_parse_password(const struct parse_data *data,
 	return 0;
 }
 
-
+#ifndef NO_CONFIG_WRITE
 static char * wpa_config_write_password(const struct parse_data *data,
 					struct wpa_ssid *ssid)
 {
@@ -1169,6 +1170,7 @@  static char * wpa_config_write_password(const struct parse_data *data,
 
 	return buf;
 }
+#endif /* NO_CONFIG_WRITE */
 #endif /* IEEE8021X_EAPOL */
 
 
@@ -2067,7 +2069,7 @@  int wpa_config_set_quoted(struct wpa_ssid *ssid, const char *var,
 	return ret;
 }
 
-
+#ifndef NO_CONFIG_WRITE
 /**
  * wpa_config_get_all - Get all options from network configuration
  * @ssid: Pointer to network configuration data
@@ -2129,8 +2131,6 @@  err:
 	return NULL;
 }
 
-
-#ifndef NO_CONFIG_WRITE
 /**
  * wpa_config_get - Get a variable in network configuration
  * @ssid: Pointer to network configuration data
diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
index 7585b86..0a931d6 100644
--- a/wpa_supplicant/wpas_glue.c
+++ b/wpa_supplicant/wpas_glue.c
@@ -35,6 +35,7 @@  static void wpa_supplicant_set_config_blob(void *ctx,
 {
 	struct wpa_supplicant *wpa_s = ctx;
 	wpa_config_set_blob(wpa_s->conf, blob);
+#ifndef CONFIG_NO_CONFIG_WRITE
 	if (wpa_s->conf->update_config) {
 		int ret = wpa_config_write(wpa_s->confname, wpa_s->conf);
 		if (ret) {
@@ -42,6 +43,7 @@  static void wpa_supplicant_set_config_blob(void *ctx,
 				   "blob set");
 		}
 	}
+#endif /* CONFIG_NO_CONFIG_WRITE */
 }
 
 
@@ -770,6 +772,7 @@  static void wpa_supplicant_set_anon_id(void *ctx, const u8 *id, size_t len)
 			return;
 	}
 
+#ifndef CONFIG_NO_CONFIG_WRITE
 	if (wpa_s->conf->update_config) {
 		res = wpa_config_write(wpa_s->confname, wpa_s->conf);
 		if (res) {
@@ -777,6 +780,7 @@  static void wpa_supplicant_set_anon_id(void *ctx, const u8 *id, size_t len)
 				   "anonymous_id update");
 		}
 	}
+#endif /* CONFIG_NO_CONFIG_WRITE */
 }
 #endif /* IEEE8021X_EAPOL */
 
@@ -798,8 +802,10 @@  int wpa_supplicant_init_eapol(struct wpa_supplicant *wpa_s)
 	ctx->eapol_done_cb = wpa_supplicant_notify_eapol_done;
 	ctx->eapol_send = wpa_supplicant_eapol_send;
 	ctx->set_wep_key = wpa_eapol_set_wep_key;
+#ifndef CONFIG_NO_CONFIG_BLOBS
 	ctx->set_config_blob = wpa_supplicant_set_config_blob;
 	ctx->get_config_blob = wpa_supplicant_get_config_blob;
+#endif /* CONFIG_NO_CONFIG_BLOBS */
 	ctx->aborted_cached = wpa_supplicant_aborted_cached;
 	ctx->opensc_engine_path = wpa_s->conf->opensc_engine_path;
 	ctx->pkcs11_engine_path = wpa_s->conf->pkcs11_engine_path;