From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <supreeth.venkatesh@arm.com>
Received-SPF: Pass (sender SPF authorized) identity=mailfrom;
 client-ip=217.140.101.70; helo=foss.arm.com;
 envelope-from=supreeth.venkatesh@arm.com; receiver=edk2-devel@lists.01.org 
Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])
 by ml01.01.org (Postfix) with ESMTP id 8F09A21194888
 for <edk2-devel@lists.01.org>; Tue, 27 Nov 2018 19:12:53 -0800 (PST)
Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])
 by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2E1512379;
 Tue, 27 Nov 2018 19:12:53 -0800 (PST)
Received: from supven01-VirtualBox (unknown [10.119.48.141])
 by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id CEBFE3F575;
 Tue, 27 Nov 2018 19:12:52 -0800 (PST)
Message-ID: <46d12bf2ce3f55cf0c5de425dfe079ca4a0c3f49.camel@arm.com>
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
To: Lokesh B V <lokesh.bv@arm.com>, edk2-devel@lists.01.org
Date: Tue, 27 Nov 2018 21:12:49 -0600
In-Reply-To: <1543341035-22358-1-git-send-email-lokesh.bv@arm.com>
References: <1543341035-22358-1-git-send-email-lokesh.bv@arm.com>
X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.1 
Mime-Version: 1.0
Subject: Re: [PATCH 2/2] [edk2-test][PATCH v2] SctPkg/Tools: Fix incorrect line ending detection by GenBin tool
X-BeenThere: edk2-devel@lists.01.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: EDK II Development  <edk2-devel.lists.01.org>
List-Unsubscribe: <https://lists.01.org/mailman/options/edk2-devel>,
 <mailto:edk2-devel-request@lists.01.org?subject=unsubscribe>
List-Archive: <http://lists.01.org/pipermail/edk2-devel/>
List-Post: <mailto:edk2-devel@lists.01.org>
List-Help: <mailto:edk2-devel-request@lists.01.org?subject=help>
List-Subscribe: <https://lists.01.org/mailman/listinfo/edk2-devel>,
 <mailto:edk2-devel-request@lists.01.org?subject=subscribe>
X-List-Received-Date: Wed, 28 Nov 2018 03:12:53 -0000
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

Same general comments apply (given in first one in the series).

On Tue, 2018-11-27 at 23:20 +0530, Lokesh B V wrote:
> Some windows editors uses "\r\n" for line feed. While processing uefi
> testcase
> info file, the GenBin tool logic to skip line feed doesn't consider
> the presence
> of carraige return(\r) in line feed. So this results in incorrect 
Please fix the commit message with "carriage return" (same comment as 
given by Philippe)
> format error.
> 
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Lokesh B V <lokesh.bv@arm.com>
> 
> Signed-off-by: Lokesh B V <lokesh.bv@arm.com>
> ---
>  uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c b/uefi-
> sct/SctPkg/Tools/Source/GenBin/GenBin.c
> index 61bb35b..4eaefcc 100644
> --- a/uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c
> +++ b/uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c
> @@ -2,6 +2,7 @@
>  
>    Copyright 2006 - 2010 Unified EFI, Inc.<BR>
>    Copyright (c) 2010 Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2018 ARM Ltd. All rights reserved.<BR>
>  
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of
> the BSD License
> @@ -176,6 +177,7 @@ Trim (
>    for (Index1 = 0; Index1 < Length; Index1++) {
>      if ((String[Index1] != ' ' ) &&
>          (String[Index1] != '\t') &&
> +        (String[Index1] != '\r') &&
>          (String[Index1] != '\n')) {
>        break;
>      }
This Trim function is ridden with bugs. Use the linux kernel
implementation to avoid bugs. Lets fix this properly. something like
below.

char *
Trim (
  char        *s
  )
{
  size_t size;
  char *end;

  size = strlen(s);

  if (!size)
    return s;

  end = s + size - 1;
  while (end >= s && isspace(*end))
    end--;

  *(end + 1) = '\0';

  while (*s && isspace(*s))
    s++;

  return s;
}

> @@ -193,6 +195,7 @@ Trim (
>    for (Index1 = 0; Index1 < Length; Index1++) {
>      if ((String[Length - 1 - Index1] != ' ' ) &&
>          (String[Length - 1 - Index1] != '\t') &&
> +        (String[Length - 1 - Index1] != '\r') &&
>          (String[Length - 1 - Index1] != '\n')) {
>        break;
>      }

Finally, Getline function should be altered as below to properly skip
empty lines and comment lines like below (or similar).

  for ( ; ; ) {
    //
    // Get a string from the profile
    //
    Result = fgets (String, MAX_LINE_LENGTH, Profile);
    if (Result == NULL) {
      return -1;
    }

    //
    // Remove the beginning and ending space characters
    //
    String = Trim (Result);

    //
    // Skip the empty line and comment line
    //
    if ((String[0] == '\0') ||
        (String[0] == '#' )) {
      continue;
    }

    (*LineNo)++;
  }