Patchwork Fixes the build error while generating option_table.h

login
register
about
Submitter Vikram Narayanan
Date 2011-05-08 17:54:41
Message ID <4DC6D8E1.50103@gmail.com>
Download mbox | patch
Permalink /patch/2957/
State Superseded
Headers show

Comments

Vikram Narayanan - 2011-05-08 17:54:41
Hi, 
I was facing a compilation error while the make is trying to generate the option_table.h from cmos.layout file. 
Unfortunately the error info generated doesn't tell where the problem is. While going through the source, I found the error producing lines.
Below is the code snippet.
<code>
if (!is_ident((char *)ce->name)) {
			fprintf(stderr,
				"Error - Name %s is an invalid identifier in line\n %s\n",
				ce->name, line);
			exit(1);
		}
</code>
Since the ce->name and line doesn't contain any thing, the make just showed me this.
"Error - Name is an invalid identifier in line"
After spending a little time I found that the line endings in the file cmos.layout are the one that is causing issues. (Think I have opened the file in windows.)
A little change in the code can make the user happy and he doesn't need to worry about the line endings. (If this isn't a good idea, may be we can give the user a more intuitive error. something like "please check your line endings". If you want it this way, I will post the patch for that too).
Please share your views.

Signed-off by: Vikram Narayanan <vikram186@gmail.com>
---

-
Thanks,
Vikram
Peter Stuge - 2011-05-08 18:01:14
Vikram Narayanan wrote:
> I found that the line endings in the file cmos.layout are the one
> that is causing issues.

Good find.


> (Think I have opened the file in windows.)

Please be careful.


> A little change in the code can make the user happy and he doesn't
> need to worry about the line endings. (If this isn't a good idea,
> may be we can give the user a more intuitive error. something like
> "please check your line endings". If you want it this way, I will
> post the patch for that too). Please share your views.

I think it's a good idea to accept \r\n in addition to \n, but..


> +++ a/util/options/build_opt_tbl.c	Sun May  8 22:34:34 2011
> @@ -372,7 +372,7 @@
>  
>  		/* skip commented and blank lines */
>  		if(line[0]=='#') continue;
> -		if(line[strspn(line," ")]=='\n') continue;
> +		if(line[0]=='\n' || line[0]=='\r') continue;

..your patch changes the semantics in another way; with your patch a
line containing only space will no longer be treated the same way by
the program. I don't think this is so good. Maybe:

if(!strncmp(line[strspn(line," ")],"\r\n",2) continue;


//Peter
Patrick Georgi - 2011-05-08 18:06:23
Am 08.05.2011 20:01, schrieb Peter Stuge:
> if(!strncmp(line[strspn(line," ")],"\r\n",2) continue;
More like:
char val=line[strspn(line," ")];
if (val=='#' || val=='\n' || val=='\r') continue;

This has the benefit of handling
"    # comment"
and mac-style newlines, too.


Patrick
Peter Stuge - 2011-05-08 18:13:06
Patrick Georgi wrote:
> Am 08.05.2011 20:01, schrieb Peter Stuge:
> > if(!strncmp(line[strspn(line," ")],"\r\n",2) continue;
> More like:
> char val=line[strspn(line," ")];
> if (val=='#' || val=='\n' || val=='\r') continue;
> 
> This has the benefit of handling
> "    # comment"

..which is another change in the rules. :)


> and mac-style newlines, too.

Hmm.. Can those old Macs even connect to the internet? :p


//Peter
Patrick Georgi - 2011-05-08 18:14:22
Am 08.05.2011 20:13, schrieb Peter Stuge:
>> "    # comment"
> 
> ..which is another change in the rules. :)
Fear not, this code will be removed in a not too far future anyway.
And ideally we can change the format soon afterwards.


Patrick

Patch

--- a/util/options/build_opt_tbl.c.orig	Sun May  8 22:18:54 2011
+++ a/util/options/build_opt_tbl.c	Sun May  8 22:34:34 2011
@@ -372,7 +372,7 @@ 
 
 		/* skip commented and blank lines */
 		if(line[0]=='#') continue;
-		if(line[strspn(line," ")]=='\n') continue;
+		if(line[0]=='\n' || line[0]=='\r') continue;
 		/* scan in the input data */
 		sscanf(line,"%d %d %c %d %s",
 			&ce->bit,&ce->length,&uc,&ce->config_id,&ce->name[0]);