Re: svn commit: r959837 - /harmony/enhanced/java/trunk/classlib/modules/portlib/src/main/native/a2e/unix/atoe.c

[Mark Hindess <ma...@googlemail.com>]

Please can we use idiomatic array syntax rather than pointer arithmetic.
That is, instead of:

  *(ebcdicArgv+i) = strdup(a2e_string(argv[i]));
  *(ebcdicArgv + size) = NULL;

use:

  ebcdicArgv[i] = strdup(a2e_string(argv[i]));
  ebcdicArgv[size] = NULL;

This is more consistent (for example with the use of argv[i] on the some
of the lines).

I've noticed this in a few other places and it makes code unnecessarily
difficult to read.

Thanks,
 Mark.

In message <20...@eris.apache.org>,
regisxu@apache.org writes:
>
> Author: regisxu
> Date: Fri Jul  2 03:18:15 2010
> New Revision: 959837
> 
> URL: http://svn.apache.org/viewvc?rev=959837&view=rev
> Log:
> a little refine code format of atoe_execvp and atoe_execv, no functional
> changes.
> 
> Modified:
>     harmony/enhanced/java/trunk/classlib/modules/portlib/src/main/native/a2e/
> unix/atoe.c
> 
> Modified: harmony/enhanced/java/trunk/classlib/modules/portlib/src/main/nativ
> e/a2e/unix/atoe.c
> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/module
> s/portlib/src/main/native/a2e/unix/atoe.c?rev=959837&r1=959836&r2=959837&view
> =diff
> =============================================================================
> =
> --- harmony/enhanced/java/trunk/classlib/modules/portlib/src/main/native/a2e/
> unix/atoe.c (original)
> +++ harmony/enhanced/java/trunk/classlib/modules/portlib/src/main/native/a2e/
> unix/atoe.c Fri Jul  2 03:18:15 2010
> @@ -2035,18 +2035,18 @@ atoe_execv (const char *path, char *cons
>      int size = 0, i, rc;
>  
>      /* Calculate the size of argv. argv always ends with a null pointer. */
> -    while (*(argv+size)) {
> +    while (*(argv + size)) {
>          size++;
>      }
>  
>      /* Allocate space for the new array and populate */
> -    ebcdicArgv = (char**) malloc(sizeof(char*) * (size+1));
> -    for (i=0; i<size; i++) {
> -        *(ebcdicArgv+i) = strdup(a2e_string(argv[i]));
> +    ebcdicArgv = (char**) malloc(sizeof(char*) * (size + 1));
> +    for (i = 0; i < size; i++) {
> +        *(ebcdicArgv + i) = strdup(a2e_string(argv[i]));
>      }
>  
>      /* Null terminate the new array */
> -    *(ebcdicArgv+size) = NULL;
> +    *(ebcdicArgv + size) = NULL;
>  
>      rc = execv(ebcdicPath, ebcdicArgv);
>  
> @@ -2072,18 +2072,18 @@ atoe_execvp (const char *file, char *con
>      int size = 0, i, rc;
>  
>      /* Calculate the size of argv. argv always ends with a null pointer. */
> -    while (*(argv+size)) {
> +    while (*(argv + size)) {
>          size++;
>      }
>  
>      /* Allocate space for the new array and populate */
> -    ebcdicArgv = (char**) malloc(sizeof(char*) * (size+1));
> -    for (i=0; i<size; i++) {
> -        *(ebcdicArgv+i) = strdup(a2e_string(argv[i]));
> +    ebcdicArgv = (char**) malloc(sizeof(char*) * (size + 1));
> +    for (i = 0; i < size; i++) {
> +        *(ebcdicArgv + i) = strdup(a2e_string(argv[i]));
>      }
>  
>      /* Null terminate the new array */
> -    *(ebcdicArgv+size) = NULL;
> +    *(ebcdicArgv + size) = NULL;
>  
>      rc = execvp(ebcdicFile, ebcdicArgv);
>  
> 

Re: svn commit: r959837 - /harmony/enhanced/java/trunk/classlib/modules/portlib/src/main/native/a2e/unix/atoe.c

[Regis <xu...@gmail.com>]

On 2010-07-02 15:06, Mark Hindess wrote:
>
> Please can we use idiomatic array syntax rather than pointer arithmetic.
> That is, instead of:
>
>    *(ebcdicArgv+i) = strdup(a2e_string(argv[i]));
>    *(ebcdicArgv + size) = NULL;
>
> use:
>
>    ebcdicArgv[i] = strdup(a2e_string(argv[i]));
>    ebcdicArgv[size] = NULL;
>
> This is more consistent (for example with the use of argv[i] on the some
> of the lines).
>
> I've noticed this in a few other places and it makes code unnecessarily
> difficult to read.
>
> Thanks,
>   Mark.
>

I do not have strange opinions on this things, well making code more readable is 
always valuable, so fixed in r959887. And also some minor fixes in r959886.

-- 
Best Regards,
Regis.