[PATCH 1/2] i2c: core: Remove needless structure member zero initialization

[PATCH 1/2] i2c: core: Remove needless structure member zero initialization

[Jarkko Nikula]

No need to clear one struct i2c_client member variable since memset has
already cleared all of them. Remove also one space intendation from err
label.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 987c124432c5..051aa3a811a8 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -283,7 +283,6 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	memset(&client, 0, sizeof(client));
 	client.adapter = adapter;
 	client.addr = sb->slave_address;
-	client.flags = 0;
 
 	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
 		client.flags |= I2C_CLIENT_TEN;
@@ -361,7 +360,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 
 	gsb->status = status;
 
- err:
+err:
 	ACPI_FREE(ares);
 	return ret;
 }
-- 
2.1.4

[PATCH 2/2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

[Jarkko Nikula]

sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and
produces following warning when CONFIG_FRAME_WARN is set to 1024:

drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’:
drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is
larger than 1024 bytes [-Wframe-larger-than=]

This is not critical given that kernel stack is 16 kB on x86_64 but lets
reduce the stack usage by allocating the struct i2c_client from the heap.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 051aa3a811a8..3f25d758d83e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -258,7 +258,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	struct acpi_connection_info *info = &data->info;
 	struct acpi_resource_i2c_serialbus *sb;
 	struct i2c_adapter *adapter = data->adapter;
-	struct i2c_client client;
+	struct i2c_client *client;
 	struct acpi_resource *ares;
 	u32 accessor_type = function >> 16;
 	u8 action = function & ACPI_IO_MASK;
@@ -269,6 +269,12 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	if (ACPI_FAILURE(ret))
 		return ret;
 
+	client = kzalloc(sizeof(*client), GFP_KERNEL);
+	if (!client) {
+		ret = AE_NO_MEMORY;
+		goto err;
+	}
+
 	if (!value64 || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) {
 		ret = AE_BAD_PARAMETER;
 		goto err;
@@ -280,74 +286,73 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 		goto err;
 	}
 
-	memset(&client, 0, sizeof(client));
-	client.adapter = adapter;
-	client.addr = sb->slave_address;
+	client->adapter = adapter;
+	client->addr = sb->slave_address;
 
 	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
-		client.flags |= I2C_CLIENT_TEN;
+		client->flags |= I2C_CLIENT_TEN;
 
 	switch (accessor_type) {
 	case ACPI_GSB_ACCESS_ATTRIB_SEND_RCV:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_byte(&client);
+			status = i2c_smbus_read_byte(client);
 			if (status >= 0) {
 				gsb->bdata = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_byte(&client, gsb->bdata);
+			status = i2c_smbus_write_byte(client, gsb->bdata);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_BYTE:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_byte_data(&client, command);
+			status = i2c_smbus_read_byte_data(client, command);
 			if (status >= 0) {
 				gsb->bdata = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_byte_data(&client, command,
+			status = i2c_smbus_write_byte_data(client, command,
 					gsb->bdata);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_WORD:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_word_data(&client, command);
+			status = i2c_smbus_read_word_data(client, command);
 			if (status >= 0) {
 				gsb->wdata = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_word_data(&client, command,
+			status = i2c_smbus_write_word_data(client, command,
 					gsb->wdata);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_BLOCK:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_block_data(&client, command,
+			status = i2c_smbus_read_block_data(client, command,
 					gsb->data);
 			if (status >= 0) {
 				gsb->len = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_block_data(&client, command,
+			status = i2c_smbus_write_block_data(client, command,
 					gsb->len, gsb->data);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_MULTIBYTE:
 		if (action == ACPI_READ) {
-			status = acpi_gsb_i2c_read_bytes(&client, command,
+			status = acpi_gsb_i2c_read_bytes(client, command,
 					gsb->data, info->access_length);
 			if (status > 0)
 				status = 0;
 		} else {
-			status = acpi_gsb_i2c_write_bytes(&client, command,
+			status = acpi_gsb_i2c_write_bytes(client, command,
 					gsb->data, info->access_length);
 		}
 		break;
@@ -361,6 +366,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	gsb->status = status;
 
 err:
+	kfree(client);
 	ACPI_FREE(ares);
 	return ret;
 }
-- 
2.1.4

Re: [PATCH 2/2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 4814 bytes --]

On Wed, Apr 29, 2015 at 03:44:37PM +0300, Jarkko Nikula wrote:
> sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and
> produces following warning when CONFIG_FRAME_WARN is set to 1024:
> 
> drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’:
> drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is
> larger than 1024 bytes [-Wframe-larger-than=]
> 
> This is not critical given that kernel stack is 16 kB on x86_64 but lets
> reduce the stack usage by allocating the struct i2c_client from the heap.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Besides it should be squashed with the previous patch, I'm fine with
this. Mika?

> ---
>  drivers/i2c/i2c-core.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 051aa3a811a8..3f25d758d83e 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -258,7 +258,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
>  	struct acpi_connection_info *info = &data->info;
>  	struct acpi_resource_i2c_serialbus *sb;
>  	struct i2c_adapter *adapter = data->adapter;
> -	struct i2c_client client;
> +	struct i2c_client *client;
>  	struct acpi_resource *ares;
>  	u32 accessor_type = function >> 16;
>  	u8 action = function & ACPI_IO_MASK;
> @@ -269,6 +269,12 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
>  	if (ACPI_FAILURE(ret))
>  		return ret;
>  
> +	client = kzalloc(sizeof(*client), GFP_KERNEL);
> +	if (!client) {
> +		ret = AE_NO_MEMORY;
> +		goto err;
> +	}
> +
>  	if (!value64 || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) {
>  		ret = AE_BAD_PARAMETER;
>  		goto err;
> @@ -280,74 +286,73 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
>  		goto err;
>  	}
>  
> -	memset(&client, 0, sizeof(client));
> -	client.adapter = adapter;
> -	client.addr = sb->slave_address;
> +	client->adapter = adapter;
> +	client->addr = sb->slave_address;
>  
>  	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
> -		client.flags |= I2C_CLIENT_TEN;
> +		client->flags |= I2C_CLIENT_TEN;
>  
>  	switch (accessor_type) {
>  	case ACPI_GSB_ACCESS_ATTRIB_SEND_RCV:
>  		if (action == ACPI_READ) {
> -			status = i2c_smbus_read_byte(&client);
> +			status = i2c_smbus_read_byte(client);
>  			if (status >= 0) {
>  				gsb->bdata = status;
>  				status = 0;
>  			}
>  		} else {
> -			status = i2c_smbus_write_byte(&client, gsb->bdata);
> +			status = i2c_smbus_write_byte(client, gsb->bdata);
>  		}
>  		break;
>  
>  	case ACPI_GSB_ACCESS_ATTRIB_BYTE:
>  		if (action == ACPI_READ) {
> -			status = i2c_smbus_read_byte_data(&client, command);
> +			status = i2c_smbus_read_byte_data(client, command);
>  			if (status >= 0) {
>  				gsb->bdata = status;
>  				status = 0;
>  			}
>  		} else {
> -			status = i2c_smbus_write_byte_data(&client, command,
> +			status = i2c_smbus_write_byte_data(client, command,
>  					gsb->bdata);
>  		}
>  		break;
>  
>  	case ACPI_GSB_ACCESS_ATTRIB_WORD:
>  		if (action == ACPI_READ) {
> -			status = i2c_smbus_read_word_data(&client, command);
> +			status = i2c_smbus_read_word_data(client, command);
>  			if (status >= 0) {
>  				gsb->wdata = status;
>  				status = 0;
>  			}
>  		} else {
> -			status = i2c_smbus_write_word_data(&client, command,
> +			status = i2c_smbus_write_word_data(client, command,
>  					gsb->wdata);
>  		}
>  		break;
>  
>  	case ACPI_GSB_ACCESS_ATTRIB_BLOCK:
>  		if (action == ACPI_READ) {
> -			status = i2c_smbus_read_block_data(&client, command,
> +			status = i2c_smbus_read_block_data(client, command,
>  					gsb->data);
>  			if (status >= 0) {
>  				gsb->len = status;
>  				status = 0;
>  			}
>  		} else {
> -			status = i2c_smbus_write_block_data(&client, command,
> +			status = i2c_smbus_write_block_data(client, command,
>  					gsb->len, gsb->data);
>  		}
>  		break;
>  
>  	case ACPI_GSB_ACCESS_ATTRIB_MULTIBYTE:
>  		if (action == ACPI_READ) {
> -			status = acpi_gsb_i2c_read_bytes(&client, command,
> +			status = acpi_gsb_i2c_read_bytes(client, command,
>  					gsb->data, info->access_length);
>  			if (status > 0)
>  				status = 0;
>  		} else {
> -			status = acpi_gsb_i2c_write_bytes(&client, command,
> +			status = acpi_gsb_i2c_write_bytes(client, command,
>  					gsb->data, info->access_length);
>  		}
>  		break;
> @@ -361,6 +366,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
>  	gsb->status = status;
>  
>  err:
> +	kfree(client);
>  	ACPI_FREE(ares);
>  	return ret;
>  }
> -- 
> 2.1.4
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

[Mika Westerberg]

On Tue, May 12, 2015 at 09:05:05PM +0200, Wolfram Sang wrote:
> On Wed, Apr 29, 2015 at 03:44:37PM +0300, Jarkko Nikula wrote:
> > sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and
> > produces following warning when CONFIG_FRAME_WARN is set to 1024:
> > 
> > drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’:
> > drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is
> > larger than 1024 bytes [-Wframe-larger-than=]
> > 
> > This is not critical given that kernel stack is 16 kB on x86_64 but lets
> > reduce the stack usage by allocating the struct i2c_client from the heap.
> > 
> > Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> Besides it should be squashed with the previous patch, I'm fine with
> this. Mika?

No objections from me,

Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

[PATCH v2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

[Jarkko Nikula]

sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and
produces following warning when CONFIG_FRAME_WARN is set to 1024:

drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’:
drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is
larger than 1024 bytes [-Wframe-larger-than=]

This is not critical given that kernel stack is 16 kB on x86_64 but lets
reduce the stack usage by allocating the struct i2c_client from the heap.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
v2:
Ack from Mika added.
"-	client.flags = 0;" removal squashed here and err label not touched.
---
 drivers/i2c/i2c-core.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 987c124432c5..ea60c1c89eb4 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -258,7 +258,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	struct acpi_connection_info *info = &data->info;
 	struct acpi_resource_i2c_serialbus *sb;
 	struct i2c_adapter *adapter = data->adapter;
-	struct i2c_client client;
+	struct i2c_client *client;
 	struct acpi_resource *ares;
 	u32 accessor_type = function >> 16;
 	u8 action = function & ACPI_IO_MASK;
@@ -269,6 +269,12 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	if (ACPI_FAILURE(ret))
 		return ret;
 
+	client = kzalloc(sizeof(*client), GFP_KERNEL);
+	if (!client) {
+		ret = AE_NO_MEMORY;
+		goto err;
+	}
+
 	if (!value64 || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) {
 		ret = AE_BAD_PARAMETER;
 		goto err;
@@ -280,75 +286,73 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 		goto err;
 	}
 
-	memset(&client, 0, sizeof(client));
-	client.adapter = adapter;
-	client.addr = sb->slave_address;
-	client.flags = 0;
+	client->adapter = adapter;
+	client->addr = sb->slave_address;
 
 	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
-		client.flags |= I2C_CLIENT_TEN;
+		client->flags |= I2C_CLIENT_TEN;
 
 	switch (accessor_type) {
 	case ACPI_GSB_ACCESS_ATTRIB_SEND_RCV:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_byte(&client);
+			status = i2c_smbus_read_byte(client);
 			if (status >= 0) {
 				gsb->bdata = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_byte(&client, gsb->bdata);
+			status = i2c_smbus_write_byte(client, gsb->bdata);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_BYTE:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_byte_data(&client, command);
+			status = i2c_smbus_read_byte_data(client, command);
 			if (status >= 0) {
 				gsb->bdata = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_byte_data(&client, command,
+			status = i2c_smbus_write_byte_data(client, command,
 					gsb->bdata);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_WORD:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_word_data(&client, command);
+			status = i2c_smbus_read_word_data(client, command);
 			if (status >= 0) {
 				gsb->wdata = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_word_data(&client, command,
+			status = i2c_smbus_write_word_data(client, command,
 					gsb->wdata);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_BLOCK:
 		if (action == ACPI_READ) {
-			status = i2c_smbus_read_block_data(&client, command,
+			status = i2c_smbus_read_block_data(client, command,
 					gsb->data);
 			if (status >= 0) {
 				gsb->len = status;
 				status = 0;
 			}
 		} else {
-			status = i2c_smbus_write_block_data(&client, command,
+			status = i2c_smbus_write_block_data(client, command,
 					gsb->len, gsb->data);
 		}
 		break;
 
 	case ACPI_GSB_ACCESS_ATTRIB_MULTIBYTE:
 		if (action == ACPI_READ) {
-			status = acpi_gsb_i2c_read_bytes(&client, command,
+			status = acpi_gsb_i2c_read_bytes(client, command,
 					gsb->data, info->access_length);
 			if (status > 0)
 				status = 0;
 		} else {
-			status = acpi_gsb_i2c_write_bytes(&client, command,
+			status = acpi_gsb_i2c_write_bytes(client, command,
 					gsb->data, info->access_length);
 		}
 		break;
@@ -362,6 +366,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
 	gsb->status = status;
 
  err:
+	kfree(client);
 	ACPI_FREE(ares);
 	return ret;
 }
-- 
2.1.4

Re: [PATCH v2] i2c: core: Reduce stack size of acpi_i2c_space_handler()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Wed, May 20, 2015 at 04:36:52PM +0300, Jarkko Nikula wrote:
> sizeof(struct i2c_client) is 1088 bytes on a CONFIG_X86_64=y build and
> produces following warning when CONFIG_FRAME_WARN is set to 1024:
> 
> drivers/i2c/i2c-core.c: In function ‘acpi_i2c_space_handler’:
> drivers/i2c/i2c-core.c:367:1: warning: the frame size of 1152 bytes is
> larger than 1024 bytes [-Wframe-larger-than=]
> 
> This is not critical given that kernel stack is 16 kB on x86_64 but lets
> reduce the stack usage by allocating the struct i2c_client from the heap.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Applied to for-next, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] i2c: core: Remove needless structure member zero initialization

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

On Wed, Apr 29, 2015 at 03:44:36PM +0300, Jarkko Nikula wrote:
> No need to clear one struct i2c_client member variable since memset has
> already cleared all of them. Remove also one space intendation from err
> label.

Ehrm, why change this now when you remove the code block in the next
patch anyhow? :)

Also, the indentation thing is very likely intentional. Sadly, I can't
remember why at this moment, but I remember that when I tried to grep
something I understood why some people put a space in front of labels

> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  drivers/i2c/i2c-core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 987c124432c5..051aa3a811a8 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -283,7 +283,6 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
>  	memset(&client, 0, sizeof(client));
>  	client.adapter = adapter;
>  	client.addr = sb->slave_address;
> -	client.flags = 0;
>  
>  	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
>  		client.flags |= I2C_CLIENT_TEN;
> @@ -361,7 +360,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address command,
>  
>  	gsb->status = status;
>  
> - err:
> +err:
>  	ACPI_FREE(ares);
>  	return ret;
>  }
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] i2c: core: Remove needless structure member zero initialization

[Jarkko Nikula]

On 05/12/2015 10:03 PM, Wolfram Sang wrote:
> On Wed, Apr 29, 2015 at 03:44:36PM +0300, Jarkko Nikula wrote:
>> No need to clear one struct i2c_client member variable since memset has
>> already cleared all of them. Remove also one space intendation from err
>> label.
>
> Ehrm, why change this now when you remove the code block in the next
> patch anyhow? :)
>
Actually second patch is not removing any other code blocks than memset 
so my 2 seconds thinking said lets put that client.flags = 0; removal to 
another patch especially after finding the err label indentation, i.e. 
spring cleanings first and then the actual patch :-)

> Also, the indentation thing is very likely intentional. Sadly, I can't
> remember why at this moment, but I remember that when I tried to grep
> something I understood why some people put a space in front of labels
>
Probably it makes code look better in some cases when there are multiple 
labels but here it didn't look consistent with the rest of the file.

I don't mind squashing this into second patch or keep them separate. I'd 
like to resend anyway because I noticed s/intendation/indentation/ 
misspelling in the commit log here.

-- 
Jarkko