mini-os: C programming

mini-os: C programming

[PUCCETTI Armand]

2 questions on the code of mini-os (the one in XEN 3.0.3):



1. In xen-3.0.3/extras/mini-os/mm.c:52 is a declaration of stack

extern char *stack;

but in file xen-3.0.3/extras/mini-os/arch/x86/setup.c:48, there is also 
a decl of stack:

char stack[8192];

If one dereferences the variable stack, which is apparently not the case 
yet, it gives surely a segfault!


2. In file xen-3.0.3/extras/mini-os/gnttab.c:140: the const variable 
gnttabop_error_msgs
is declared as

static const char *gnttabop_error_msgs[] = GNTTABOP_error_msgs;

shouldn't that instead be declared:

static const char * const gnttabop_error_msgs[] = GNTTABOP_error_msgs;
?
(BTW, even the first const is useless as there are consts in 
GNTTABOP_error_msgs)

Re: mini-os: C programming

[Keir Fraser]

On 15/3/07 13:39, "PUCCETTI Armand" <armand.puccetti@cea.fr> wrote:

> extern char *stack;

Yes, this one is bogus.

> 2. In file xen-3.0.3/extras/mini-os/gnttab.c:140: the const variable
> gnttabop_error_msgs
> is declared as
> 
> static const char *gnttabop_error_msgs[] = GNTTABOP_error_msgs;
> 
> shouldn't that instead be declared:
> 
> static const char * const gnttabop_error_msgs[] = GNTTABOP_error_msgs;

It doesn't really matter, does it? Personally I hate scattering 'const' all
over the place.

 -- Keir

Re: mini-os: C programming

[PUCCETTI Armand]

Keir Fraser a écrit :
> On 15/3/07 13:39, "PUCCETTI Armand" <armand.puccetti@cea.fr> wrote:
>
>   
>> extern char *stack;
>>     
>
> Yes, this one is bogus.
>
>   
>> 2. In file xen-3.0.3/extras/mini-os/gnttab.c:140: the const variable
>> gnttabop_error_msgs
>> is declared as
>>
>> static const char *gnttabop_error_msgs[] = GNTTABOP_error_msgs;
>>
>> shouldn't that instead be declared:
>>
>> static const char * const gnttabop_error_msgs[] = GNTTABOP_error_msgs;
>>     
>
> It doesn't really matter, does it? Personally I hate scattering 'const' all
> over the place.
>
>  -- Keir
>
>
>   
It doesn't cause any crash probably, but according to the semantics that you
wish gnttabop_error_msgs to have, all pointers of this array _should_ be 
constant
(I guess you're not going to change the error messages dynamically)
and not only the strings refered by these pointers. The type of both 
declarations
is given by cdecl:

$ cdecl
char * const gnttabop_error_msgs[] ;
declare gnttabop_error_msgs as array of const pointer to char;

const char * gnttabop_error_msgs[];
declare gnttabop_error_msgs as array of pointer to const char;

RE: mini-os: C programming

[Petersson, Mats]

 

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com 
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of 
> PUCCETTI Armand
> Sent: 15 March 2007 15:12
> To: Keir Fraser
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] mini-os: C programming
> 
> Keir Fraser a écrit :
> > On 15/3/07 13:39, "PUCCETTI Armand" <armand.puccetti@cea.fr> wrote:
> >
> >   
> >> extern char *stack;
> >>     
> >
> > Yes, this one is bogus.
> >
> >   
> >> 2. In file xen-3.0.3/extras/mini-os/gnttab.c:140: the 
> const variable
> >> gnttabop_error_msgs
> >> is declared as
> >>
> >> static const char *gnttabop_error_msgs[] = GNTTABOP_error_msgs;
> >>
> >> shouldn't that instead be declared:
> >>
> >> static const char * const gnttabop_error_msgs[] = 
> GNTTABOP_error_msgs;
> >>     
> >
> > It doesn't really matter, does it? Personally I hate 
> scattering 'const' all
> > over the place.
> >
> >  -- Keir
> >
> >
> >   
> It doesn't cause any crash probably, but according to the 
> semantics that you
> wish gnttabop_error_msgs to have, all pointers of this array 
> _should_ be 
> constant
> (I guess you're not going to change the error messages dynamically)
> and not only the strings refered by these pointers. The type of both 
> declarations
> is given by cdecl:
> 
> $ cdecl
> char * const gnttabop_error_msgs[] ;
> declare gnttabop_error_msgs as array of const pointer to char;
> 
> const char * gnttabop_error_msgs[];
> declare gnttabop_error_msgs as array of pointer to const char;

There are two different reasons to use const:
1. To tell the compiler to generate warnings when you change a const, e.g. 

const int x = 7;

...
x = 42;
... 
Should give a warning about changing the value of a const. 

2. To give the compiler a clue to optimize code better. For example, the initial declaraton of x = 7 can be replaced by the constant value 7 by the compiler whenever it finds this as a more useful way to do things. Similarly, a const character array passed into a function can be expected to not have changed by the called function (so strcpy() for example isn't allowed to change the second parameter, as it's a const). 

I'm pretty sure most usages of const in Xen is for the purpose of the latter. Of course, getting a compiler warning when you've modified something that is supposed to be const isn't a bad thing. 

In the case of error messages, it doesn't really make much sense whether the compiler can generate the perfect code for it or not - it's going to take much longer to actually put it in a human readable place (such as the screen or serial port) than it takes to find the actual error message anyways. And I think this is what Keir ment with "it doesn't matter". 

Yes, setting the error message value for error 6 will not generate a compiler warning (as long as the pointer to char you set it to is a "constant"), but it's not a very likely thing to be done in the code anyways. 

--
Mats
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
> 
>