Skip to contentSkip to conversation

wpa-supplicant: add options to enable the new DBus interface

Buildroot New Sven Neumann 2 participants 13 messages Sep 13, 2012 – Sep 19, 2012 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

Hi,

I am resending this patch rebased to current master because it did not get any attention so far. However I would still like it to be included in buildroot.

Regards,
Sven

#1 lore raw

PATCH 1/1 wpa-supplicant: add options to enable the new DBus interface

Allow to configure the DBus interfaces that the wpa_supplicant binary should support (old or new or both). Also allow to enable introspection support on the new DBus interface.

---
 package/wpa_supplicant/Config.in         |   19 ++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk |   42 ++++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 5 deletions(-)
package/wpa_supplicant/Config.in +19 -0
index d7cefe3..d41913e 100644
@@ -30,6 +30,25 @@ config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT
 	  (optionally, with WPS); this links in parts of hostapd functionality
 	  into wpa_supplicant.
 
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD
+	bool "Enable support for old DBus control interface"
+        depends on BR2_PACKAGE_DBUS
+	help
+	  Enable support for old DBus control interface
+	  (fi.epitest.hostap.WPASupplicant).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW
+	bool "Enable support for new DBus control interface"
+        depends on BR2_PACKAGE_DBUS
+	help
+	  Enable support for new DBus control interface (fi.w1.wpa_supplicant1).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_INTROSPECTION
+	bool "Introspection support for new DBus control interface"
+        depends on BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW
+	help
+	  Add introspection support for new DBus control interface.
+
 config BR2_PACKAGE_WPA_SUPPLICANT_WPS
 	bool "Enable support for WPS"
 	help
package/wpa_supplicant/wpa_supplicant.mk +37 -5

62 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 1/1 wpa-supplicant: add options to enable the new DBus interface

Quoted 5 lines · Sven Neumann

Allow to configure the DBus interfaces that the wpa_supplicant binary should support (old or new or both). Also allow to enable introspection support on the new DBus interface.

Since nobody is reviewing it even after your resend, I'll do it.
However, I'm not familiar with wpa_supplicant and I don't have internet access now, so I may miss some things.

I have a few comments, but nothing major (the spaces are the
only thing that really have to be fixed). So

Quoted 10 lines
---
  package/wpa_supplicant/Config.in         |   19 ++++++++++++++
  package/wpa_supplicant/wpa_supplicant.mk |   42 ++++++++++++++++++++++++++----
  2 files changed, 56 insertions(+), 5 deletions(-)
package/wpa_supplicant/Config.in +0 -0
index d7cefe3..d41913e 100644

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT · line 35

Quoted 6 lines
package/wpa_supplicant/Config.in +3 -0
@@ -30,6 +30,25 @@ config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT
  	  (optionally, with WPS); this links in parts of hostapd functionality
  	  into wpa_supplicant.
 
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD
+	bool "Enable support for old DBus control interface"
+        depends on BR2_PACKAGE_DBUS

Use 1 tab instead of 8 spaces (below as well).

Also, I think it's better to make this
select BR2_PACKAGE_DBUS
depends on BR2_TOOLCHAIN_HAS_THREADS

Also, since this only installs a .service file, and it used to be
always enabled before, I would make at least one of the two DBus interfaces enabled by default.

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT · line 47

Quoted 12 lines
package/wpa_supplicant/Config.in +12 -0
+	help
+	  Enable support for old DBus control interface
+	  (fi.epitest.hostap.WPASupplicant).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW
+	bool "Enable support for new DBus control interface"
+        depends on BR2_PACKAGE_DBUS
+	help
+	  Enable support for new DBus control interface (fi.w1.wpa_supplicant1).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_INTROSPECTION
+	bool "Introspection support for new DBus control interface"

Just write 'Introspection support'. This config will be indented
under the 'new DBus control interface', so that's clear enough.

Quoted 22 lines
package/wpa_supplicant/Config.in +4 -0
+        depends on BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW
+	help
+	  Add introspection support for new DBus control interface.
+
  config BR2_PACKAGE_WPA_SUPPLICANT_WPS
  	bool "Enable support for WPS"
  	help
package/wpa_supplicant/wpa_supplicant.mk +2 -1
index d8c916b..207eec1 100644
@@ -10,7 +10,8 @@ WPA_SUPPLICANT_LICENSE = GPLv2/BSD-3c
  WPA_SUPPLICANT_LICENSE_FILES = README
  WPA_SUPPLICANT_CONFIG = $(WPA_SUPPLICANT_DIR)/wpa_supplicant/.config
  WPA_SUPPLICANT_SUBDIR = wpa_supplicant
-WPA_SUPPLICANT_DBUS_SERVICE = fi.epitest.hostap.WPASupplicant
+WPA_SUPPLICANT_DBUS_OLD_SERVICE = fi.epitest.hostap.WPASupplicant
+WPA_SUPPLICANT_DBUS_NEW_SERVICE = fi.w1.wpa_supplicant1
  WPA_SUPPLICANT_CFLAGS = $(TARGET_CFLAGS) -I$(STAGING_DIR)/usr/include/libnl3/
  WPA_SUPPLICANT_LDFLAGS = $(TARGET_LDFLAGS)
 

package/wpa_supplicant/wpa_supplicant.mk ifeq ($(BR2_PACKAGE_DBUS),y) · line 91

Quoted 8 lines
package/wpa_supplicant/wpa_supplicant.mk +3 -1
@@ -84,9 +85,41 @@ ifeq ($(BR2_PACKAGE_DBUS),y)
  	WPA_SUPPLICANT_MAKE_ENV = \
  		PKG_CONFIG_SYSROOT_DIR="$(STAGING_DIR)"	\
  		PKG_CONFIG_PATH="$(STAGING_DIR)/usr/lib/pkgconfig"
-define WPA_SUPPLICANT_DBUS_CONFIG
+
+ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD),y)
+define WPA_SUPPLICANT_DBUS_OLD_CONFIG
  	$(SED) 's/\(#\)\(CONFIG_CTRL_IFACE_DBUS=\)/\2/' $(WPA_SUPPLICANT_CONFIG)

The \(\) around the # are redundant.

Since there are a few sed scripts here, it would be nicer to write:

ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD),y) WPA_SUPPLICANT_CONFIG_EDIT += -e 's/#\(CONFIG_CTRL_IFACE_DBUS=\)/\1/' define WPA_SUPPLICANT_INSTALL_DBUS_OLD
...
endef
endif

...

ifneq ($(WPA_SUPPLICANT_CONFIG_EDIT),)
define WPA_SUPPLICANT_DBUS_CONFIG

     $(SED) $(WPA_SUPPLICANT_CONFIG_EDIT) $(WPA_SUPPLICANT_CONFIG)

endef
endif

Regards,
Arnout

Quoted 46 lines
package/wpa_supplicant/wpa_supplicant.mk +32 -3
  endef
+define WPA_SUPPLICANT_INSTALL_DBUS_OLD
+	$(INSTALL) -D \
+	  $(@D)/wpa_supplicant/dbus/$(WPA_SUPPLICANT_DBUS_OLD_SERVICE).service \
+	  $(TARGET_DIR)/usr/share/dbus-1/system-services/$(WPA_SUPPLICANT_DBUS_OLD_SERVICE).service
+endef
+endif
+
+ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW),y)
+define WPA_SUPPLICANT_DBUS_NEW_CONFIG
+	$(SED) 's/\(#\)\(CONFIG_CTRL_IFACE_DBUS_NEW=\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+endef
+define WPA_SUPPLICANT_INSTALL_DBUS_NEW
+	$(INSTALL) -D \
+	  $(@D)/wpa_supplicant/dbus/$(WPA_SUPPLICANT_DBUS_NEW_SERVICE).service \
+	  $(TARGET_DIR)/usr/share/dbus-1/system-services/$(WPA_SUPPLICANT_DBUS_NEW_SERVICE).service
+endef
+endif
+
+ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_INTROSPECTION),y)
+define WPA_SUPPLICANT_DBUS_INTROSPECTION_CONFIG
+	$(SED) 's/\(#\)\(CONFIG_CTRL_IFACE_DBUS_INTRO=\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+endef
+endif
+
+define WPA_SUPPLICANT_DBUS_CONFIG
+	$(WPA_SUPPLICANT_DBUS_OLD_CONFIG)
+	$(WPA_SUPPLICANT_DBUS_NEW_CONFIG)
+	$(WPA_SUPPLICANT_DBUS_INTROSPECTION_CONFIG)
+endef
+
  endif
 
  define WPA_SUPPLICANT_CONFIGURE_CMDS
@@ -131,9 +164,8 @@ define WPA_SUPPLICANT_INSTALL_DBUS
  	$(INSTALL) -D \
  	  $(@D)/wpa_supplicant/dbus/dbus-wpa_supplicant.conf \
  	  $(TARGET_DIR)/etc/dbus-1/system.d/wpa_supplicant.conf
-	$(INSTALL) -D \
-	  $(@D)/wpa_supplicant/dbus/$(WPA_SUPPLICANT_DBUS_SERVICE).service \
-	  $(TARGET_DIR)/usr/share/dbus-1/system-services/$(WPA_SUPPLICANT_DBUS_SERVICE).service
+	$(WPA_SUPPLICANT_INSTALL_DBUS_OLD)
+	$(WPA_SUPPLICANT_INSTALL_DBUS_NEW)
  endef
  endif
 
Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#3 lore raw

[Buildroot] wpa_supplicant changes

Hi,

here's a new patch series based on comments from Arnout. The first patch is the old one with whitespace issues fixed. The second patch removes some redundant escaping from the various sed commands in wpa_supplicant.mk. The third patch tries to make the whole thing more readable by introducing macros for editing the .config file. Please review and comment.

Regards, Sven

#4 lore raw

PATCH 1/3 wpa-supplicant: add options to enable the new DBus interface

Allow to configure the DBus interfaces that the wpa_supplicant binary should support (old or new or both). Also allow to enable introspection support on the new DBus interface.

---
 package/wpa_supplicant/Config.in         |   19 ++++++++++++++
 package/wpa_supplicant/wpa_supplicant.mk |   42 ++++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 5 deletions(-)
package/wpa_supplicant/Config.in +19 -0
index d7cefe3..95d9620 100644
@@ -30,6 +30,25 @@ config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT
 	  (optionally, with WPS); this links in parts of hostapd functionality
 	  into wpa_supplicant.
 
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD
+	bool "Enable support for old DBus control interface"
+	depends on BR2_PACKAGE_DBUS
+	help
+	  Enable support for old DBus control interface
+	  (fi.epitest.hostap.WPASupplicant).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW
+	bool "Enable support for new DBus control interface"
+        depends on BR2_PACKAGE_DBUS
+	help
+	  Enable support for new DBus control interface (fi.w1.wpa_supplicant1).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_INTROSPECTION
+	bool "Introspection support"
+        depends on BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW
+	help
+	  Add introspection support for new DBus control interface.
+
 config BR2_PACKAGE_WPA_SUPPLICANT_WPS
 	bool "Enable support for WPS"
 	help
package/wpa_supplicant/wpa_supplicant.mk +37 -5

62 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#5 lore raw

PATCH 1/3 wpa-supplicant: add options to enable the new DBus interface

Quoted 18 lines · Sven Neumann

Allow to configure the DBus interfaces that the wpa_supplicant binary should support (old or new or both). Also allow to enable introspection support on the new DBus interface.

---
  package/wpa_supplicant/Config.in         |   19 ++++++++++++++
  package/wpa_supplicant/wpa_supplicant.mk |   42 ++++++++++++++++++++++++++----
  2 files changed, 56 insertions(+), 5 deletions(-)
package/wpa_supplicant/Config.in +0 -0
index d7cefe3..95d9620 100644
@@ -30,6 +30,25 @@ config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT
  	  (optionally, with WPS); this links in parts of hostapd functionality
  	  into wpa_supplicant.
 

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT · line 42

Quoted 10 lines
package/wpa_supplicant/Config.in +10 -0
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD
+	bool "Enable support for old DBus control interface"
+	depends on BR2_PACKAGE_DBUS
+	help
+	  Enable support for old DBus control interface
+	  (fi.epitest.hostap.WPASupplicant).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW
+	bool "Enable support for new DBus control interface"
+        depends on BR2_PACKAGE_DBUS

Still 8 spaces instead of 1 tab.

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT · line 48

Quoted 6 lines
package/wpa_supplicant/Config.in +6 -0
+	help
+	  Enable support for new DBus control interface (fi.w1.wpa_supplicant1).
+
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_INTROSPECTION
+	bool "Introspection support"
+        depends on BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW

Still 8 spaces instead of 1 tab.

When I said the whitespace issue was the only thing had to be fixed, I didn't
mean that my other comments could be ignored :-)

[snip]

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#6 lore raw

PATCH 1/3 wpa-supplicant: add options to enable the new DBus interface

Quoted 2 lines · Arnout Vandecappelle

When I said the whitespace issue was the only thing had to be fixed, I didn't
mean that my other comments could be ignored

Sorry, I hadn't noticed your third patch...

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#7 lore raw

PATCH 2/3 wpa_supplicant: remove redundant escaping from sed commands

In reply to Sven Neumann
---
 package/wpa_supplicant/wpa_supplicant.mk |   44 +++++++++++++++---------------
 1 file changed, 22 insertions(+), 22 deletions(-)
package/wpa_supplicant/wpa_supplicant.mk +22 -22
index 207eec1..e8d55d3 100644
@@ -28,15 +28,15 @@ endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_EAP),y)
 define WPA_SUPPLICANT_EAP_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_EAP_AKA.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_FAST.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_GPSK.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_IKEV2.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_PAX.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_PSK.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_SAKE.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_SIM.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_TNC.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_AKA.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_FAST.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_GPSK.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_IKEV2.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_PAX.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_PSK.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_SAKE.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_SIM.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_TNC.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 endef
 else
 define WPA_SUPPLICANT_EAP_CONFIG
@@ -52,30 +52,30 @@ endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_WPS),y)
 define WPA_SUPPLICANT_WPS_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_WPS.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_WPS.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 endef
 endif
 
 define WPA_SUPPLICANT_LIBTOMMATH_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_INTERNAL_LIBTOMMATH.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_INTERNAL_LIBTOMMATH.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 endef
 
 # Try to use openssl or gnutls if it's already available
 ifeq ($(BR2_PACKAGE_OPENSSL),y)
 	WPA_SUPPLICANT_DEPENDENCIES += openssl
 define WPA_SUPPLICANT_TLS_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_TLS=openssl\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_EAP_PWD.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_TLS=openssl\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_EAP_PWD.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 endef
 else
 ifeq ($(BR2_PACKAGE_GNUTLS),y)
 	WPA_SUPPLICANT_DEPENDENCIES += gnutls
 define WPA_SUPPLICANT_TLS_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_TLS=\).*/\2gnutls/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_TLS=\).*/\1gnutls/' $(WPA_SUPPLICANT_CONFIG)
 endef
 else
 define WPA_SUPPLICANT_TLS_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_TLS=\).*/\2internal/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_TLS=\).*/\1internal/' $(WPA_SUPPLICANT_CONFIG)
 endef
 endif
 endif
@@ -88,7 +88,7 @@ ifeq ($(BR2_PACKAGE_DBUS),y)
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD),y)
 define WPA_SUPPLICANT_DBUS_OLD_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_CTRL_IFACE_DBUS=\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_CTRL_IFACE_DBUS=\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 endef
 define WPA_SUPPLICANT_INSTALL_DBUS_OLD
 	$(INSTALL) -D \
@@ -99,7 +99,7 @@ endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW),y)
 define WPA_SUPPLICANT_DBUS_NEW_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_CTRL_IFACE_DBUS_NEW=\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_CTRL_IFACE_DBUS_NEW=\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 endef
 define WPA_SUPPLICANT_INSTALL_DBUS_NEW
 	$(INSTALL) -D \
@@ -110,7 +110,7 @@ endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_INTROSPECTION),y)
 define WPA_SUPPLICANT_DBUS_INTROSPECTION_CONFIG
-	$(SED) 's/\(#\)\(CONFIG_CTRL_IFACE_DBUS_INTRO=\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_CTRL_IFACE_DBUS_INTRO=\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 endef
 endif
 
@@ -124,10 +124,10 @@ endif
 
 define WPA_SUPPLICANT_CONFIGURE_CMDS
 	cp $(@D)/wpa_supplicant/defconfig $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_IEEE80211N.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_IEEE80211R.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_INTERWORKING.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(#\)\(CONFIG_DELAYED_MIC.*\)/\2/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_IEEE80211N.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_IEEE80211R.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_INTERWORKING.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(SED) 's/#\(CONFIG_DELAYED_MIC.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
 	$(SED) 's/\(CONFIG_DRIVER_ATMEL\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
 	$(SED) 's/\(CONFIG_SMARTCARD\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
 	$(WPA_SUPPLICANT_LIBTOMMATH_CONFIG)
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH 2/3 wpa_supplicant: remove redundant escaping from sed commands

Quoted 1 line · Sven Neumann

(visual inspection only)

Although the third patch makes this one pretty much redundant...
Still, it doesn't hurt to do it in separate steps.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#9 lore raw

PATCH 3/3 wpa_supplicant: introduce macros for editing the .config file

In reply to Sven Neumann

Make the substitutions on the .config file more readable by introducing macros to enable, disable certain options similar to the macros we use for editing KConfig files.

---
 package/wpa_supplicant/wpa_supplicant.mk |   62 ++++++++++++++++++------------
 1 file changed, 37 insertions(+), 25 deletions(-)
package/wpa_supplicant/wpa_supplicant.mk +37 -25
index e8d55d3..5337e5f 100644
@@ -15,49 +15,61 @@ WPA_SUPPLICANT_DBUS_NEW_SERVICE = fi.w1.wpa_supplicant1
 WPA_SUPPLICANT_CFLAGS = $(TARGET_CFLAGS) -I$(STAGING_DIR)/usr/include/libnl3/
 WPA_SUPPLICANT_LDFLAGS = $(TARGET_LDFLAGS)
 
+define WPA_SUPPLICANT_CONFIG_DISABLE
+	$(SED) 's/^\($(1).*\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
+endef
+
+define WPA_SUPPLICANT_CONFIG_ENABLE
+	$(SED) 's/#\($(1).*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+endef
+
+define WPA_SUPPLICANT_CONFIG_SET
+	echo '$(1)=y' >> $(WPA_SUPPLICANT_CONFIG)
+endef
+
 ifeq ($(BR2_PACKAGE_LIBNL),y)
 	WPA_SUPPLICANT_DEPENDENCIES += libnl
 define WPA_SUPPLICANT_LIBNL_CONFIG
-	echo 'CONFIG_LIBNL32=y' >>$(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_SET,CONFIG_LIBNL32)
 endef
 else
 define WPA_SUPPLICANT_LIBNL_CONFIG
-	$(SED) 's/^\(CONFIG_DRIVER_NL80211.*\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_DISABLE,CONFIG_DRIVER_NL80211)
 endef
 endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_EAP),y)
 define WPA_SUPPLICANT_EAP_CONFIG
-	$(SED) 's/#\(CONFIG_EAP_AKA.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_FAST.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_GPSK.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_IKEV2.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_PAX.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_PSK.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_SAKE.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_SIM.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_TNC.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_AKA)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_FAST)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_GPSK)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_IKEV2)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_PAX)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_PSK)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_SAKE)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_SIM)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_TNC)
 endef
 else
 define WPA_SUPPLICANT_EAP_CONFIG
-	$(SED) 's/^\(CONFIG_EAP.*\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_DISABLE,CONFIG_EAP)
 endef
 endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT),y)
 define WPA_SUPPLICANT_AP_CONFIG
-	echo 'CONFIG_AP=y' >>$(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_SET,CONFIG_AP)
 endef
 endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_WPS),y)
 define WPA_SUPPLICANT_WPS_CONFIG
-	$(SED) 's/#\(CONFIG_WPS.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_WPS)
 endef
 endif
 
 define WPA_SUPPLICANT_LIBTOMMATH_CONFIG
-	$(SED) 's/#\(CONFIG_INTERNAL_LIBTOMMATH.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_INTERNAL_LIBTOMMATH)
 endef
 
 # Try to use openssl or gnutls if it's already available
@@ -65,7 +77,7 @@ ifeq ($(BR2_PACKAGE_OPENSSL),y)
 	WPA_SUPPLICANT_DEPENDENCIES += openssl
 define WPA_SUPPLICANT_TLS_CONFIG
 	$(SED) 's/#\(CONFIG_TLS=openssl\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_EAP_PWD.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_EAP_PWD)
 endef
 else
 ifeq ($(BR2_PACKAGE_GNUTLS),y)
@@ -88,7 +100,7 @@ ifeq ($(BR2_PACKAGE_DBUS),y)
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD),y)
 define WPA_SUPPLICANT_DBUS_OLD_CONFIG
-	$(SED) 's/#\(CONFIG_CTRL_IFACE_DBUS=\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_CTRL_IFACE_DBUS=)
 endef
 define WPA_SUPPLICANT_INSTALL_DBUS_OLD
 	$(INSTALL) -D \
@@ -99,7 +111,7 @@ endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_NEW),y)
 define WPA_SUPPLICANT_DBUS_NEW_CONFIG
-	$(SED) 's/#\(CONFIG_CTRL_IFACE_DBUS_NEW=\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_CTRL_IFACE_DBUS_NEW)
 endef
 define WPA_SUPPLICANT_INSTALL_DBUS_NEW
 	$(INSTALL) -D \
@@ -110,7 +122,7 @@ endif
 
 ifeq ($(BR2_PACKAGE_WPA_SUPPLICANT_DBUS_INTROSPECTION),y)
 define WPA_SUPPLICANT_DBUS_INTROSPECTION_CONFIG
-	$(SED) 's/#\(CONFIG_CTRL_IFACE_DBUS_INTRO=\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_CTRL_IFACE_DBUS_INTRO)
 endef
 endif
 
@@ -124,12 +136,12 @@ endif
 
 define WPA_SUPPLICANT_CONFIGURE_CMDS
 	cp $(@D)/wpa_supplicant/defconfig $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_IEEE80211N.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_IEEE80211R.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_INTERWORKING.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/#\(CONFIG_DELAYED_MIC.*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(CONFIG_DRIVER_ATMEL\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
-	$(SED) 's/\(CONFIG_SMARTCARD\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_IEEE80211N)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_IEEE80211R)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_INTERWORKING)
+	$(call WPA_SUPPLICANT_CONFIG_ENABLE,CONFIG_DELAYED_MIC)
+	$(call WPA_SUPPLICANT_CONFIG_DISABLE,CONFIG_DRIVER_ATMEL)
+	$(call WPA_SUPPLICANT_CONFIG_DISABLE,CONFIG_SMARTCARD)
 	$(WPA_SUPPLICANT_LIBTOMMATH_CONFIG)
 	$(WPA_SUPPLICANT_TLS_CONFIG)
 	$(WPA_SUPPLICANT_EAP_CONFIG)
Signature · 1 line
1.7.9.5
#10 lore raw

PATCH 3/3 wpa_supplicant: introduce macros for editing the .config file

Quoted 5 lines · Sven Neumann

Make the substitutions on the .config file more readable by introducing macros to enable, disable certain options similar to the macros we use for editing KConfig files.

Nice!

Did you use a sed (or editor) regexp to do the substitution? If yes,
it makes sense to document that regexp in the commit message. That helps to review it, and can also be useful for others who are inspired to do a similar modification.

Quoted 26 lines
---
  package/wpa_supplicant/wpa_supplicant.mk |   62 ++++++++++++++++++------------
  1 file changed, 37 insertions(+), 25 deletions(-)
package/wpa_supplicant/wpa_supplicant.mk +12 -0
index e8d55d3..5337e5f 100644
@@ -15,49 +15,61 @@ WPA_SUPPLICANT_DBUS_NEW_SERVICE = fi.w1.wpa_supplicant1
  WPA_SUPPLICANT_CFLAGS = $(TARGET_CFLAGS) -I$(STAGING_DIR)/usr/include/libnl3/
  WPA_SUPPLICANT_LDFLAGS = $(TARGET_LDFLAGS)
 
+define WPA_SUPPLICANT_CONFIG_DISABLE
+	$(SED) 's/^\($(1).*\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
+endef
+
+define WPA_SUPPLICANT_CONFIG_ENABLE
+	$(SED) 's/#\($(1).*\)/\1/' $(WPA_SUPPLICANT_CONFIG)
+endef
+
+define WPA_SUPPLICANT_CONFIG_SET
+	echo '$(1)=y'>>  $(WPA_SUPPLICANT_CONFIG)
+endef
+
  ifeq ($(BR2_PACKAGE_LIBNL),y)
  	WPA_SUPPLICANT_DEPENDENCIES += libnl

package/wpa_supplicant/wpa_supplicant.mk WPA_SUPPLICANT_DBUS_NEW_SERVICE = fi.w1.wpa_supplicant1 · line 39

Quoted 10 lines
package/wpa_supplicant/wpa_supplicant.mk +2 -2
  define WPA_SUPPLICANT_LIBNL_CONFIG
-	echo 'CONFIG_LIBNL32=y'>>$(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_SET,CONFIG_LIBNL32)
  endef
  else
  define WPA_SUPPLICANT_LIBNL_CONFIG
-	$(SED) 's/^\(CONFIG_DRIVER_NL80211.*\)/#\1/' $(WPA_SUPPLICANT_CONFIG)
+	$(call WPA_SUPPLICANT_CONFIG_DISABLE,CONFIG_DRIVER_NL80211)
  endef
  endif

While you're at it, I'd like to avoid calling sed dozens of times,
by writing it like this:

----
WPA_SUPPLICANT_CONFIG_DISABLE = -e 's/^\($(1).*\)/#\1/' WPA_SUPPLICANT_CONFIG_ENABLE = -e 's/#\($(1).*\)/\1/'
WPA_SUPPLICANT_CONFIG_SET = -e '1i$(1)=y'

ifeq ($(BR2_PACKAGE_LIBNL),y)
WPA_SUPPLICANT_DEPENDENCIES += libnl
WPA_SUPPLICANT_CONFIG_EDITS += $(call WPA_SUPPLICANT_CONFIG_SET,CONFIG_LIBNL32) else
WPA_SUPPLICANT_CONFIG_EDITS += $(call WPA_SUPPLICANT_CONFIG_DISABLE,CONFIG_LIBNL32) endif

...

define WPA_SUPPLICANT_CONFIGURE_CMDS

	cp $(@D)/wpa_supplicant/defconfig $(WPA_SUPPLICANT_CONFIG)
	$(SED) $(WPA_SUPPLICANT_CONFIG_EDITS) $(WPA_SUPPLICANT_CONFIG)

endef
----

Of course, using 'sed 1i...' instead of 'echo ... >>' means those options
will be at the beginning rather than at the end, but I don't think that should make a difference.

I just love hacking make, so here's another alternative:

----
ifeq ($(BR2_PACKAGE_LIBNL),y)
WPA_SUPPLICANT_DEPENDENCIES += libnl
WPA_SUPPLICANT_CONFIG_SET += CONFIG_LIBNL32
else
WPA_SUPPLICANT_CONFIG_DISABLE += CONFIG_LIBNL32
endif

...

define WPA_SUPPLICANT_CONFIGURE_CMDS

	cp $(@D)/wpa_supplicant/defconfig $(WPA_SUPPLICANT_CONFIG)
	$(SED) $(patsubst %,-e 's/^\(%\)/#\1/',$(WPA_SUPPLICANT_CONFIG_DISABLE)) \
	       $(patsubst %,-e 's/^#\(%\)/\1/',$(WPA_SUPPLICANT_CONFIG_ENABLE)) \
	       $(patsubst %,-e '1i%=y',$(WPA_SUPPLICANT_CONFIG_SET)) \
	       $(WPA_SUPPLICANT_CONFIG)

endef
----

Regards,
Arnout

[snip]

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

A message that is not in the archive (sent off-list or not yet received): [email protected]

#12 lore raw

PATCH 1/1 wpa-supplicant: add options to enable the new DBus interface

Hi,

thanks a lot for your review. I'll make the changes you suggested, but I am not sure about the following:

Quoted 6 lines · Arnout Vandecappelle

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT · line 35

Quoted 3 lines
package/wpa_supplicant/Config.in +3 -0
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD
+	bool "Enable support for old DBus control interface"
+        depends on BR2_PACKAGE_DBUS

Use 1 tab instead of 8 spaces (below as well).

Quoted 7 lines

Also, I think it's better to make this
select BR2_PACKAGE_DBUS
depends on BR2_TOOLCHAIN_HAS_THREADS

Also, since this only installs a .service file, and it used to be
always enabled before, I would make at least one of the two DBus interfaces enabled by default.

If I follow this suggestion and enable one of the DBus interfaces by default, then selecting wpa-supplicant would select DBus, which it did not do before. Is that really desired?

Regards,
Sven

#13 lore raw

PATCH 1/1 wpa-supplicant: add options to enable the new DBus interface

Quoted 17 lines · Sven Neumann

Hi,

thanks a lot for your review. I'll make the changes you suggested, but I am not sure about the following:

Quoted 10 lines · Arnout Vandecappelle

package/wpa_supplicant/Config.in config BR2_PACKAGE_WPA_SUPPLICANT_AP_SUPPORT · line 35

Quoted 3 lines
package/wpa_supplicant/Config.in +3 -0
+config BR2_PACKAGE_WPA_SUPPLICANT_DBUS_OLD
+	bool "Enable support for old DBus control interface"
+        depends on BR2_PACKAGE_DBUS

Use 1 tab instead of 8 spaces (below as well).

Also, I think it's better to make this
select BR2_PACKAGE_DBUS
depends on BR2_TOOLCHAIN_HAS_THREADS

Quoted 7 lines
Quoted 3 lines

Also, since this only installs a .service file, and it used to be
always enabled before, I would make at least one of the two DBus interfaces enabled by default.

If I follow this suggestion and enable one of the DBus interfaces by default, then selecting wpa-supplicant would select DBus, which it did not do before. Is that really desired?

Actually, on second thought, it makes more sense to depend on dbus
after all: .service files are installed if dbus is enabled. That's also the old behaviour.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F