[PATCH] Fixes the build error while generating option_table.h

[Vikram Narayanan]

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 
---
--- 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]);

-
Thanks,
Vikram

[PATCH] Fixes the build error while generating option_table.h

[Peter Stuge]

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

[PATCH] Fixes the build error while generating option_table.h

[Patrick Georgi]

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

[PATCH] Fixes the build error while generating option_table.h

[Peter Stuge]

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

[PATCH] Fixes the build error while generating option_table.h

[Patrick Georgi]

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