public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>
To: Minnow Ware <minnowware@outlook.com>,
	"Jin, Eric" <eric.jin@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Jin, Eric" <eric.jin@intel.com>
Subject: Re: [edk2-test][PATCH v2] SctPkg/Tools: Fix incorrect line ending detection by GenBin tool
Date: Fri, 29 Mar 2019 14:34:32 +0000	[thread overview]
Message-ID: <AM4PR08MB2788DFC3F3152CC00C9C85D9805A0@AM4PR08MB2788.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <AM5PR04MB2993EEB3AADA67A33912D39DC15A0@AM5PR04MB2993.eurprd04.prod.outlook.com>

Thanks Kilian.
There was no issue. This patch was sent in error.
However, I will look into this feedback - "The GenBin.c should be extended by a manual BIN2TXT translation."

Supreeth


From: Minnow Ware <minnowware@outlook.com>
Sent: Friday, March 29, 2019 9:29 AM
To: Jin, Eric <eric.jin@intel.com>; Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>; edk2-devel@lists.01.org
Cc: Jin, Eric <eric.jin@intel.com>
Subject: RE: [edk2] [edk2-test][PATCH v2] SctPkg/Tools: Fix incorrect line ending detection by GenBin tool

Hi Supreeth,

theoretically the standard C implementation of fopen(char *filename, char *mode)
"can" support BINARY and TEXT mode. This is specified here http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf, "7.19.2 Streams"

In praxis only Windows-/DOS platforms supports a BINARY ( w/ "b") and a TEXT mode (w/o "b").
This is valid for all C-library implementations on that platforms.

In textmode all "\r\n"-sequences in the file are automatically (by the C Library) translated to "\n" only,
when reading the file to the buffer.
And on write operations each "\n" is transformed to "\r\n".

On UNIX-systems with or without "b"-inary flag do not affect the data read/written.

Since the profile file is opened correctly in text mode (Line 107 https://github.com/tianocore/edk2-test/blob/master/uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c),
the issue tells me, you are running on a UNIX system, dealing with Windows text files.

The GenBin.c should be extended by a manual BIN2TXT translation.

Best Regrads,
Kilian

https://github.com/MinnowWare


From: Jin, Eric<mailto:eric.jin@intel.com>
Sent: Friday, March 29, 2019 09:31 AM
To: Supreeth Venkatesh<mailto:supreeth.venkatesh@arm.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Jin, Eric<mailto:eric.jin@intel.com>
Subject: Re: [edk2] [edk2-test][PATCH v2] SctPkg/Tools: Fix incorrect line ending detection by GenBin tool

Hi Supreeth,

This patch can't be applied to the lasted code base SHA-1: 33022e.
One patch from <Lokesh B V> may already fix the issue. Please check again.

Best Regards
Eric

-----Original Message-----
From: edk2-devel <edk2-devel-bounces@lists.01.org<mailto:edk2-devel-bounces@lists.01.org>> On Behalf Of Supreeth Venkatesh
Sent: Friday, March 29, 2019 7:08 AM
To: supreeth.venkatesh@arm.com<mailto:supreeth.venkatesh@arm.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: [edk2] [edk2-test][PATCH v2] SctPkg/Tools: Fix incorrect line ending detection by GenBin tool

From: Lokesh B V <lokesh.bv@arm.com<mailto:lokesh.bv@arm.com>>

Some windows editors uses "\r\n" for line feed. While processing uefi sct 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 format error.

Fixed this issue by changing logic of Trim and Getline functions used in GenBin source code.

Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com<mailto:supreeth.venkatesh@arm.com>>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Lokesh B V <lokesh.bv@arm.com<mailto:lokesh.bv@arm.com>>
---
Changes since v1:
* Adopted changes suggested by Supreeth.
* modified Copyright content

 uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c | 58 +++++++++++++---------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c b/uefi-sct/SctPkg/Tools/Source/GenBin/GenBin.c
index 61bb35b..6d8e844 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 @@ -32,6 +33,7 @@ Abstract:
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <ctype.h>

 //
 // Definitions
@@ -49,7 +51,7 @@ PrintUsage (
   void
   );

-void
+char *
 Trim (
   char        *String
   );
@@ -159,50 +161,42 @@ PrintUsage (
 }


-void
+char *
 Trim (
   char        *String
   )
 {
-  int   Index1;
-  int   Index2;
   int   Length;
+  char  *end;

   Length = strlen (String);

-  //
-  // Remove the space characters from the beginning of this string
-  //
-  for (Index1 = 0; Index1 < Length; Index1++) {
-    if ((String[Index1] != ' ' ) &&
-        (String[Index1] != '\t') &&
-        (String[Index1] != '\n')) {
-      break;
-    }
-  }
-
-  for (Index2 = Index1; Index2 < Length; Index2++) {
-    String[Index2 - Index1] = String[Index2];
+  if (!Length) {
+    return String;
   }

-  Length = Length - Index1;
+  end = String + Length - 1;

   //
   // Remove the space characters from the end of this string
   //
-  for (Index1 = 0; Index1 < Length; Index1++) {
-    if ((String[Length - 1 - Index1] != ' ' ) &&
-        (String[Length - 1 - Index1] != '\t') &&
-        (String[Length - 1 - Index1] != '\n')) {
-      break;
-    }
+  while (end >= String && isspace (*end)) {
+    end--;
   }

-  String[Length - Index1] = '\0';
+  *(end + 1) = '\0';
+
+  //
+  // Remove the space characters from the beginning of this string  //
+ while (*String && isspace (*String)) {
+    String++;
+  }

   //
   // Done
   //
+  return String;
 }


@@ -227,20 +221,20 @@ GetLine (
       return -1;
     }

-    (*LineNo)++;
-
     //
     // Remove the beginning and ending space characters
     //
-    Trim (String);
+    String = Trim (Result);

     //
-    // Ignore the empty line and comment line
+    // Skip the empty line and comment line
     //
-    if ((String[0] != '\0') &&
-        (String[0] != '#' )) {
-      break;
+    if ((String[0] == '\0') ||
+        (String[0] == '#' )) {
+      continue;
     }
+
+    (*LineNo)++;
   }

   //
--
2.7.4


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


  reply	other threads:[~2019-03-29 14:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-28  9:29 [edk2-test][PATCH] Framework/Include: allow usage with EFI version 2.7 Lokesh B V
2018-11-28  9:29 ` [edk2-test][PATCH] SctPkg/UEFI: Fix invalid GUID value format error Lokesh B V
     [not found]   ` <f59c45a7de154bd3706c54b73a48aa975dbfbf96.camel@arm.com>
2018-11-28 22:28     ` Supreeth Venkatesh
2018-11-28 22:29   ` Supreeth Venkatesh
2018-11-28  9:30 ` [edk2-test][PATCH v2] SctPkg/build: Add support for GenBin tool build Lokesh B V
2018-11-28 22:30   ` Supreeth Venkatesh
2018-11-28  9:30 ` [edk2-test][PATCH] Add *.o and *.d pattern to .gitignore Lokesh B V
2018-11-28 22:29   ` Supreeth Venkatesh
2018-11-28  9:30 ` [edk2-test][PATCH v2] SctPkg/Tools: Fix incorrect line ending detection by GenBin tool Lokesh B V
2018-11-28 22:29   ` Supreeth Venkatesh
2019-03-29  8:31   ` Jin, Eric
2019-03-29 14:11     ` Supreeth Venkatesh
2019-03-29 14:28     ` Minnow Ware
2019-03-29 14:34       ` Supreeth Venkatesh [this message]
2018-11-28 22:30 ` [edk2-test][PATCH] Framework/Include: allow usage with EFI version 2.7 Supreeth Venkatesh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM4PR08MB2788DFC3F3152CC00C9C85D9805A0@AM4PR08MB2788.eurprd08.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox