Re: Mainly noise

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Walter Briscoe
I started a further conversation with Bram about diagnostics flagged by
PC-lint in vim 7. Point 1.i below was prompted by that and needs to be
flagged to vim-dev. It seems OK to post this to both Bram and vim-dev.

In message <[hidden email]> of Fri, 1 Jul
2005 15:30:13 in , Bram Moolenaar <[hidden email]> writes
Bram,
I have the latest build which I have fed through lint. I am now down to
a few messages - some seem spurious to me.

1) There seem a couple of glitches in changes I prompted to if-ole.cpp:
i) SetKeyAndValue() definition lost a star.
ii) 778 et. seq. have runs of spaces where a tab would be more usual.

--- src/if_ole.cpp.0    Sun Jul  3 10:46:14 2005
+++ src/if_ole.cpp      Sun Jul  3 12:37:52 2005
@@ -651,7 +651,7 @@
 }

 // Create a key and set its value
-static void SetKeyAndValue(const char *key, const char subkey, const char *value)
+static void SetKeyAndValue(const char *key, const char *subkey, const char *value)
 {
     HKEY hKey;
     char buffer[1024];
@@ -778,4 +778,77 @@
        cf = NULL;
     }
 }


1a) --- Module:   if_ole.cpp
        typeinfo->Release();
                           ^
if_ole.cpp(189) : Warning 1551: Function may throw exception '...' in destructor 'CVim::~CVim(void)'

I don't know enough C++ to address this. I think it probably spurious.


2) --- Module:   ex_getln.c

During Specific Walk:
  File ex_getln.c line 2968: ExpandOne(?, 0, 0, 0, 5?)
  File ex_getln.c line 3149: ExpandFromContext(?, 0, [1], [1], 0)
  File ex_getln.c line 4029: ExpandRTDir(0, [1], [1], !=0)
ex_getln.c(4286) : Warning 418: Passing null pointer to function 'strlen(const char *)', arg. no. 1 [Reference: file ex_getln.c: lines 2968,
 3148, 4029]

PC-lint value-tracking is insufficiently sophisticated. There are
several such spurious diagnostics.


3) --- Module:   main.c
    while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
                                     ^
farsi.c(1954) : Warning 564: variable 'p' depends on order of evaluation

I think this message is probably spurious. Note the spelling error. I
assume you are holding off putting spell checks in mode lines until a
later stage in 7.0 development. The following patch may be better:
--- src/farsi.c.0       Sun Jun 13 19:25:28 2004
+++ src/farsi.c Fri Jul  1 10:53:06 2005
@@ -1950,8 +1950,8 @@

     p = ibuf;

-    /* Find the boundry of the search path */
-    while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
+    /* Find the boundary of the search path */
+    while (++p, ((p = vim_strchr(p, '/')) != NULL) && p[-1] == '\\')
        ;

     if (p == NULL)


4) --- Module:   gui.c
                                                     &general_beval_cb, NULL);
                                                                      ^
gui.c(568) : Warning 546: Suspicious use of &

You already rejected this one. It is the only one of its kind apart from
its two neighbours. It means: "Why use &? You already have a pointer".


5) --- Module:   gui_w32.c
    add_long(lStyle);
#... *((LPDWORD)p)++ = (lStyle)
                   ^
gui_w32.c(2875) : Error 52: Expected an lvalue

A cast loses lvalueness in C while retaining it in C++
I don't have a fix I like for this and similarly-featured macros.
--
Walter Briscoe
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

James Widman

On Jul 3, 2005, at 11:30 AM, Walter Briscoe wrote:

> I started a further conversation with Bram about diagnostics  
> flagged by
> PC-lint in vim 7. Point 1.i below was prompted by that and needs to be
> flagged to vim-dev. It seems OK to post this to both Bram and vim-dev.
>
> In message <[hidden email]> of Fri, 1 Jul
> 2005 15:30:13 in , Bram Moolenaar <[hidden email]> writes
> Bram,
> I have the latest build which I have fed through lint. I am now  
> down to
> a few messages - some seem spurious to me.
>
> 1) There seem a couple of glitches in changes I prompted to if-
> ole.cpp:
> i) SetKeyAndValue() definition lost a star.
> ii) 778 et. seq. have runs of spaces where a tab would be more usual.
>
> --- src/if_ole.cpp.0    Sun Jul  3 10:46:14 2005
> +++ src/if_ole.cpp      Sun Jul  3 12:37:52 2005
> @@ -651,7 +651,7 @@
>  }
>
>  // Create a key and set its value
> -static void SetKeyAndValue(const char *key, const char subkey,  
> const char *value)
> +static void SetKeyAndValue(const char *key, const char *subkey,  
> const char *value)
>  {
>      HKEY hKey;
>      char buffer[1024];
> @@ -778,4 +778,77 @@
>         cf = NULL;
>      }
>  }
>
>
> 1a) --- Module:   if_ole.cpp
>         typeinfo->Release();
>                            ^
> if_ole.cpp(189) : Warning 1551: Function may throw exception '...'  
> in destructor 'CVim::~CVim(void)'
>
> I don't know enough C++ to address this. I think it probably spurious.

It may be undesired in this case, but if ITypeInfo::Release is  
declared with the exception specification 'throw(...)' (or,  
equivalently, if it was declared without an exception specification),  
then this message is not spurious.  If you're sure that functions  
implicitly declared with throw(...) do not actually throw, then just  
suppress with -esym(1551,...).  Otherwise, wrap the call with try{}  
and insert a catch(...) block.  Destructors must not throw.

> 2) --- Module:   ex_getln.c
>
> During Specific Walk:
>   File ex_getln.c line 2968: ExpandOne(?, 0, 0, 0, 5?)
>   File ex_getln.c line 3149: ExpandFromContext(?, 0, [1], [1], 0)
>   File ex_getln.c line 4029: ExpandRTDir(0, [1], [1], !=0)
> ex_getln.c(4286) : Warning 418: Passing null pointer to function  
> 'strlen(const char *)', arg. no. 1 [Reference: file ex_getln.c:  
> lines 2968,
>  3148, 4029]
>
> PC-lint value-tracking is insufficiently sophisticated. There are
> several such spurious diagnostics.

At first blush this certainly seems spurious.  Walter, can you send  
me your lint configuration for vim7 (and the preprocessor output for  
that module)?  I'd like to reproduce this.

> 3) --- Module:   main.c
>     while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
>                                      ^
> farsi.c(1954) : Warning 564: variable 'p' depends on order of  
> evaluation
>
> I think this message is probably spurious.

Yes, definitely; I think we're reporting because the order of  
evaluation of operands to the assignment operator is unspecified and  
because 'p' appeared somewhere on both sides and was modified on at  
least one.  Of course, that doesn't matter in this case since the  
value of s is not being used to determine where the result of the  
expression will be stored.  Clearly, our analysis should be refined  
here.

> Note the spelling error. I
> assume you are holding off putting spell checks in mode lines until a
> later stage in 7.0 development. The following patch may be better:
> --- src/farsi.c.0       Sun Jun 13 19:25:28 2004
> +++ src/farsi.c Fri Jul  1 10:53:06 2005
> @@ -1950,8 +1950,8 @@
>
>      p = ibuf;
>
> -    /* Find the boundry of the search path */
> -    while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
> +    /* Find the boundary of the search path */
> +    while (++p, ((p = vim_strchr(p, '/')) != NULL) && p[-1] == '\\')
>         ;
>
>      if (p == NULL)
>

It's embarrassing if source code has to be modified just because  
Lint's analysis is not quite complete. Try instead using the Lint  
option:

-d"vim_strchr=/*lint --e(564)*/vim_strchr"

...which defines the function name as a macro that expands to the  
same name plus a lint comment that suppresses the message for the  
duration of the containing expression.

> 4) --- Module:   gui.c
>                                                      
> &general_beval_cb, NULL);
>                                                                        
> ^
> gui.c(568) : Warning 546: Suspicious use of &
>
> You already rejected this one. It is the only one of its kind apart  
> from
> its two neighbours. It means: "Why use &? You already have a pointer".

This one may be more an issue of personal preference.  If you  
typically use an explicit '&' with function pointer literals, then  
just suppress 568.

> 5) --- Module:   gui_w32.c
>     add_long(lStyle);
> #... *((LPDWORD)p)++ = (lStyle)
>                    ^
> gui_w32.c(2875) : Error 52: Expected an lvalue
>
> A cast loses lvalueness in C

Right...

> ...while retaining it in C++

Quoth ISO C++ section 5.4,p1:
"The result of the expression(T)cast-expression is of type T. The  
result is an lvalue if T is a reference
type, otherwise the result is an rvalue."

> I don't have a fix I like for this and similarly-featured macros.

Ditto.  I'd like to see the fix if/when someone comes up with one.  
I'm not sure that this diagnostic points to a bug that would result  
in Bad Things Happening(TM), but it's definitely a non-portable  
construct.  The Comeau compiler is one that issues an error when it  
sees code like this.

James Widman
--
Gimpel Software
http://gimpel.com

Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

George V. Reilly
James Widman wrote:

> On Jul 3, 2005, at 11:30 AM, Walter Briscoe wrote:
>
>> 1a) --- Module:   if_ole.cpp
>>         typeinfo->Release();
>>                            ^
>> if_ole.cpp(189) : Warning 1551: Function may throw exception '...'  
>> in destructor 'CVim::~CVim(void)'
>>
>> I don't know enough C++ to address this. I think it probably spurious.
>
>
> It may be undesired in this case, but if ITypeInfo::Release is  
> declared with the exception specification 'throw(...)' (or,  
> equivalently, if it was declared without an exception specification),  
> then this message is not spurious.  If you're sure that functions  
> implicitly declared with throw(...) do not actually throw, then just  
> suppress with -esym(1551,...).  Otherwise, wrap the call with try{}  
> and insert a catch(...) block.  Destructors must not throw.

By convention, COM objects are not supposed to allow exceptions to
propagate across COM interface boundaries. Furthermore, IDL doesn't
allow you to decorate functions with throw specifications. Since the
typeinfo object is created by COM's infrastructure, I'm sure it's
well-behaved. But if you have to wrap the Release in a try-catch to shut
Lint up, so be it.

--
I can't hear you, my tin ear is bothering me again.
(Get Witty Auto-Generated Signatures from http://SmartBee.org)
George V. Reilly     [hidden email]
Read my blog: http://georgevreilly.com/blog


Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Walter Briscoe
In reply to this post by James Widman
In message <[hidden email]> of Sun, 3
Jul 2005 17:07:27 in , James Widman <[hidden email]> writes

>
>On Jul 3, 2005, at 11:30 AM, Walter Briscoe wrote:
>
>> I started a further conversation with Bram about diagnostics
>>flagged by
>> PC-lint in vim 7. Point 1.i below was prompted by that and needs to be
>> flagged to vim-dev. It seems OK to post this to both Bram and vim-dev.
>>
>> In message <[hidden email]> of Fri, 1 Jul
>> 2005 15:30:13 in , Bram Moolenaar <[hidden email]> writes
>> Bram,
>> I have the latest build which I have fed through lint. I am now
>>down to
>> a few messages - some seem spurious to me.
>>

[snip]

>> 1a) --- Module:   if_ole.cpp
>>         typeinfo->Release();
>>                            ^
>> if_ole.cpp(189) : Warning 1551: Function may throw exception '...'
>>in destructor 'CVim::~CVim(void)'
>>
>> I don't know enough C++ to address this. I think it probably spurious.
>
>It may be undesired in this case, but if ITypeInfo::Release is
>declared with the exception specification 'throw(...)' (or,
>equivalently, if it was declared without an exception specification),
>then this message is not spurious.  If you're sure that functions
>implicitly declared with throw(...) do not actually throw, then just
>suppress with -esym(1551,...).  Otherwise, wrap the call with try{}
>and insert a catch(...) block.  Destructors must not throw.

I will suppress that 1551. I assume putting in code to handle an event
which never happens is futile.

>
>> 2) --- Module:   ex_getln.c
>>
>> During Specific Walk:
>>   File ex_getln.c line 2968: ExpandOne(?, 0, 0, 0, 5?)
>>   File ex_getln.c line 3149: ExpandFromContext(?, 0, [1], [1], 0)
>>   File ex_getln.c line 4029: ExpandRTDir(0, [1], [1], !=0)
>> ex_getln.c(4286) : Warning 418: Passing null pointer to function
>>'strlen(const char *)', arg. no. 1 [Reference: file ex_getln.c:
>>lines 2968,
>>  3148, 4029]
>>
>> PC-lint value-tracking is insufficiently sophisticated. There are
>> several such spurious diagnostics.
>
>At first blush this certainly seems spurious.  Walter, can you send  me
>your lint configuration for vim7 (and the preprocessor output for  that
>module)?  I'd like to reproduce this.

Done! It took a while. I nearly sent it here rather than to James.
My fallibility continues  o frighten me.

>
>> 3) --- Module:   main.c
>>     while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
>>                                      ^
>> farsi.c(1954) : Warning 564: variable 'p' depends on order of
>>evaluation
>>
>> I think this message is probably spurious.
>
>Yes, definitely; I think we're reporting because the order of
>evaluation of operands to the assignment operator is unspecified and
>because 'p' appeared somewhere on both sides and was modified on at
>least one.  Of course, that doesn't matter in this case since the
>value of s is not being used to determine where the result of the
>expression will be stored.  Clearly, our analysis should be refined
>here.

I think your analysis is adequate.

[snip]

>It's embarrassing if source code has to be modified just because
>Lint's analysis is not quite complete. Try instead using the Lint
>option:
>
>-d"vim_strchr=/*lint --e(564)*/vim_strchr"

I can do that. If the code were mine, I would code around the message.
I just looked up the C99 Annex C (informative) Sequence points. PC-lint
must have a simpler, more conservative model. The difference only bites
once in vim.

>> 4) --- Module:   gui.c
>>
>>&general_beval_cb, NULL);
>>
>>^
>> gui.c(568) : Warning 546: Suspicious use of &
>>
>> You already rejected this one. It is the only one of its kind apart
>>from
>> its two neighbours. It means: "Why use &? You already have a pointer".
>
>This one may be more an issue of personal preference.  If you
>typically use an explicit '&' with function pointer literals, then
>just suppress 568.

Quite so. That usage is not typical in vim. I prefer 1 pattern to 2 even
if they are equivalent.

>
>> 5) --- Module:   gui_w32.c
>>     add_long(lStyle);
>> #... *((LPDWORD)p)++ = (lStyle)
>>                    ^
>> gui_w32.c(2875) : Error 52: Expected an lvalue
>>
>> A cast loses lvalueness in C
>
>Right...
>
>> ...while retaining it in C++
>
>Quoth ISO C++ section 5.4,p1:
>"The result of the expression(T)cast-expression is of type T. The
>result is an lvalue if T is a reference
>type, otherwise the result is an rvalue."

Which is narrower than I thought.

>
>> I don't have a fix I like for this and similarly-featured macros.
>
>Ditto.  I'd like to see the fix if/when someone comes up with one.
>I'm not sure that this diagnostic points to a bug that would result  in
>Bad Things Happening(TM), but it's definitely a non-portable
>construct.  The Comeau compiler is one that issues an error when it
>sees code like this.

Good for Greg! On reflection, I would be inclined to use a function
add_long() and do things painfully but "correctly".
The code in question is on no critical path. Relying on this extension
seems unnecessary to me.

Thanks for the unexpected response.
--
Walter Briscoe
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Bram Moolenaar
In reply to this post by James Widman

James Widman wrote:

> > 2) --- Module:   ex_getln.c
> >
> > During Specific Walk:
> >   File ex_getln.c line 2968: ExpandOne(?, 0, 0, 0, 5?)
> >   File ex_getln.c line 3149: ExpandFromContext(?, 0, [1], [1], 0)
> >   File ex_getln.c line 4029: ExpandRTDir(0, [1], [1], !=0)
> > ex_getln.c(4286) : Warning 418: Passing null pointer to function  
> > 'strlen(const char *)', arg. no. 1 [Reference: file ex_getln.c:  
> > lines 2968,
> >  3148, 4029]
> >
> > PC-lint value-tracking is insufficiently sophisticated. There are
> > several such spurious diagnostics.
>
> At first blush this certainly seems spurious.  Walter, can you send  
> me your lint configuration for vim7 (and the preprocessor output for  
> that module)?  I'd like to reproduce this.

The NULL argument isn't used, because another argument has a specific
value that is checked in an "if".  That's nearly impossible to find in
static checks: The "mode" argument is passed the value of the "type"
variable, which was set to WILD_NEXT or WILD_PREV earlier.  Stil, a
human can figure this out, thus a program might be able to see it too.

> > 5) --- Module:   gui_w32.c
> >     add_long(lStyle);
> > #... *((LPDWORD)p)++ = (lStyle)
> >                    ^
> > gui_w32.c(2875) : Error 52: Expected an lvalue
> >
> > A cast loses lvalueness in C
>
> Right...
>
> > ...while retaining it in C++
>
> Quoth ISO C++ section 5.4,p1:
> "The result of the expression(T)cast-expression is of type T. The  
> result is an lvalue if T is a reference
> type, otherwise the result is an rvalue."
>
> > I don't have a fix I like for this and similarly-featured macros.
>
> Ditto.  I'd like to see the fix if/when someone comes up with one.  
> I'm not sure that this diagnostic points to a bug that would result  
> in Bad Things Happening(TM), but it's definitely a non-portable  
> construct.  The Comeau compiler is one that issues an error when it  
> sees code like this.

I think nearly all C compilers can handle this, since in practice it's
just incrementing a pointer by the size of what it points to.  In some C
standards it might be "undefined" though.

Problem is that there is no good workaround to avoid the warning message.

--
hundred-and-one symptoms of being an internet addict:
223. You set up a web-cam as your home's security system.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

James Widman
In reply to this post by Walter Briscoe
Hi Marcus,

On Jul 4, 2005, at 2:09 PM, Marcus Aurelius wrote:

> --- James Widman <[hidden email]> escreveu:
>
>
>>> 3) --- Module:   main.c
>>>     while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
>>>                                      ^
>>> farsi.c(1954) : Warning 564: variable 'p' depends on order of
>>> evaluation
>>>
>>> I think this message is probably spurious.
>>>
>>
>> Yes, definitely; I think we're reporting because the order of
>> evaluation of operands to the assignment operator is unspecified and
>> because 'p' appeared somewhere on both sides and was modified on at
>> least one.  Of course, that doesn't matter in this case since the
>> value of s is not being used to determine where the result of the
>> expression will be stored.  Clearly, our analysis should be refined
>> here.
>>
>
> Maybe i'm missing something, but i can't see how this could be  
> correct:
> ...(p = vim_strchr(++p, '/'))...
> p is incremented and then assigned to. Isn't this similar to "i = +
> +i"?
> I'm not an expert about sequence points, but i guess it's undefined  
> behavior...

My apologies.  I applied intuition to form an opinion about this, and  
it turns out that opinion was incorrect.

Sorry people; I'm new here.

Then again, I'm on weekend/holiday time.  FWIW. (;

Excuses, excuses...

Anyway: (i = ++i) seems, at first glance, like a safe (albeit  
questionable) expression.  One might think (as I did) that,  
regardless of which side of '=' is evaluated first,  the result is  
the same.  But ISO C99, section 6.5,p2 says:

"Between the previous and next sequence point an object shall have  
its stored value modified at most once by the evaluation of an  
expression."

So according to C99, (p = vim_strchr(++p, '/')) results in undefined  
behavior because p is modified twice.

ISO C++ has basically the same verbiage.  I don't have our copies of  
ISO C90 or the first-edition K&R C book nearby, so I don't yet know  
the situation there.

James Widman
--
Gimpel Software
http://gimpel.com

Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Bram Moolenaar

James Widman wrote:

> Anyway: (i = ++i) seems, at first glance, like a safe (albeit  
> questionable) expression.  One might think (as I did) that,  
> regardless of which side of '=' is evaluated first,  the result is  
> the same.  But ISO C99, section 6.5,p2 says:
>
> "Between the previous and next sequence point an object shall have  
> its stored value modified at most once by the evaluation of an  
> expression."
>
> So according to C99, (p = vim_strchr(++p, '/')) results in undefined  
> behavior because p is modified twice.
>
> ISO C++ has basically the same verbiage.  I don't have our copies of  
> ISO C90 or the first-edition K&R C book nearby, so I don't yet know  
> the situation there.

The problem here is the standard.  C99 is probably the worst C standard
that was ever made.  It was clearly formed by politicians, not by
practical programmers.  OK, well, that's just my opinion (a ran into a
couple of annoying problems).

That "undefined behavior" is stupid.  Of course every good-old C
programmer can predict what should and will happen.  Only very bad C
compilers would not be able to deal with it.  The standard should be
adjusted to meet what programmers do and expect, not the other way
around.

--
You can tune a file system, but you can't tuna fish
                                                        -- man tunefs

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Mike Williams
Bram Moolenaar did utter on 05/07/2005 10:29:

> James Widman wrote:
>
>
>>Anyway: (i = ++i) seems, at first glance, like a safe (albeit  
>>questionable) expression.  One might think (as I did) that,  
>>regardless of which side of '=' is evaluated first,  the result is  
>>the same.  But ISO C99, section 6.5,p2 says:
>>
>>"Between the previous and next sequence point an object shall have  
>>its stored value modified at most once by the evaluation of an  
>>expression."
>>
>>So according to C99, (p = vim_strchr(++p, '/')) results in undefined  
>>behavior because p is modified twice.
>>
>>ISO C++ has basically the same verbiage.  I don't have our copies of  
>>ISO C90 or the first-edition K&R C book nearby, so I don't yet know  
>>the situation there.
>
>
> The problem here is the standard.  C99 is probably the worst C standard
> that was ever made.  It was clearly formed by politicians, not by
> practical programmers.  OK, well, that's just my opinion (a ran into a
> couple of annoying problems).
>
> That "undefined behavior" is stupid.  Of course every good-old C
> programmer can predict what should and will happen.  Only very bad C
> compilers would not be able to deal with it.  The standard should be
> adjusted to meet what programmers do and expect, not the other way
> around.

I would hope any good-old C programmer would spot the possibility of
undefined behaviour and rewrite the expression to remove the problem.
On some processors a good optimising compiler (producing correct fast
code for well defined C) would fail with this invalid code.  Side
effects and sequence points are standard fare for any C job interview.

TTFN

Mike
--
Is there a lawyer in the house? <BLAM!!> Is there another?
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

James Widman
In reply to this post by Bram Moolenaar

On Jul 5, 2005, at 5:29 AM, Bram Moolenaar wrote:

>
> James Widman wrote:
>
>
>> Anyway: (i = ++i) seems, at first glance, like a safe (albeit
>> questionable) expression.  One might think (as I did) that,
>> regardless of which side of '=' is evaluated first,  the result is
>> the same.  But ISO C99, section 6.5,p2 says:
>>
>> "Between the previous and next sequence point an object shall have
>> its stored value modified at most once by the evaluation of an
>> expression."


FYI, this is not new text in C99.  Those exact words appear in C89/90  
section 3.3,p2.  I cannot find anything so strict in the first K&R C  
book.


>> So according to C99, (p = vim_strchr(++p, '/')) results in undefined
>> behavior because p is modified twice.
>>
>> ISO C++ has basically the same verbiage.  I don't have our copies of
>> ISO C90 or the first-edition K&R C book nearby, so I don't yet know
>> the situation there.
>>
>
> The problem here is the standard.  C99 is probably the worst C  
> standard
> that was ever made.  It was clearly formed by politicians, not by
> practical programmers.  OK, well, that's just my opinion (a ran into a
> couple of annoying problems).
>
> That "undefined behavior" is stupid.  Of course every good-old C
> programmer can predict what should and will happen.  Only very bad C
> compilers would not be able to deal with it.

I fail to see how anything could actually go wrong with (i = ++i);  
perhaps there are issues associated with optimization.  To me, a more  
likely reason for this rule is that the committee had plenty of work  
to do as it was; they were (and still are) on short schedules and  
were (and still are) volunteers.  So they probably wanted to keep the  
normative text as simple as possible.

The fact that it rendered expressions like (i = ++i) invalid was  
probably considered and deemed to have little importance since those  
expressions can easily be rewritten such that behavior is well-defined.

> The standard should be adjusted to meet what programmers do and  
> expect, not the other way around.

You'll find people who agree with you in both committees.

James Widman
--
Gimpel Software
http://gimpel.com

Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Matthew Winn
In reply to this post by Bram Moolenaar
On Tue, Jul 05, 2005 at 11:29:51AM +0200, Bram Moolenaar wrote:

>
> James Widman wrote:
>
> > Anyway: (i = ++i) seems, at first glance, like a safe (albeit  
> > questionable) expression.  One might think (as I did) that,  
> > regardless of which side of '=' is evaluated first,  the result is  
> > the same.  But ISO C99, section 6.5,p2 says:
> >
> > "Between the previous and next sequence point an object shall have  
> > its stored value modified at most once by the evaluation of an  
> > expression."
> >
> > So according to C99, (p = vim_strchr(++p, '/')) results in undefined  
> > behavior because p is modified twice.
> >
> > ISO C++ has basically the same verbiage.  I don't have our copies of  
> > ISO C90 or the first-edition K&R C book nearby, so I don't yet know  
> > the situation there.
>
> The problem here is the standard.  C99 is probably the worst C standard
> that was ever made.  It was clearly formed by politicians, not by
> practical programmers.  OK, well, that's just my opinion (a ran into a
> couple of annoying problems).
>
> That "undefined behavior" is stupid.  Of course every good-old C
> programmer can predict what should and will happen.  Only very bad C
> compilers would not be able to deal with it.  The standard should be
> adjusted to meet what programmers do and expect, not the other way
> around.

It's undefined so the compiler can do it as efficiently as possible.
On an architecture where transferring information from the stack to
memory is fast, an expression like q = func(++p) may be fastest if the
incremented value of p isn't written back to the correct memory location
until after q has been altered.  Why should either speed of execution
or speed of compilation suffer just to deal with the special case where
a programmer attempts to alter the same value two different ways in
one expression, especially as p = func(p+1) is almost certainly more
efficient than p = func(++p) anyway?

--
Matthew Winn ([hidden email])
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Bram Moolenaar
In reply to this post by Mike Williams

Mike Williams wrote:

> > James Widman wrote:
> >
> >>Anyway: (i = ++i) seems, at first glance, like a safe (albeit  
> >>questionable) expression.  One might think (as I did) that,  
> >>regardless of which side of '=' is evaluated first,  the result is  
> >>the same.  But ISO C99, section 6.5,p2 says:
> >>
> >>"Between the previous and next sequence point an object shall have  
> >>its stored value modified at most once by the evaluation of an  
> >>expression."
> >>
> >>So according to C99, (p = vim_strchr(++p, '/')) results in undefined  
> >>behavior because p is modified twice.
> >>
> >>ISO C++ has basically the same verbiage.  I don't have our copies of  
> >>ISO C90 or the first-edition K&R C book nearby, so I don't yet know  
> >>the situation there.
> >
> >
> > The problem here is the standard.  C99 is probably the worst C standard
> > that was ever made.  It was clearly formed by politicians, not by
> > practical programmers.  OK, well, that's just my opinion (a ran into a
> > couple of annoying problems).
> >
> > That "undefined behavior" is stupid.  Of course every good-old C
> > programmer can predict what should and will happen.  Only very bad C
> > compilers would not be able to deal with it.  The standard should be
> > adjusted to meet what programmers do and expect, not the other way
> > around.
>
> I would hope any good-old C programmer would spot the possibility of
> undefined behaviour and rewrite the expression to remove the problem.
> On some processors a good optimising compiler (producing correct fast
> code for well defined C) would fail with this invalid code.  Side
> effects and sequence points are standard fare for any C job interview.

I'm glad I don't have to do that job interview :-).

Of course it's not clean programming to change a pointer twice, but in
rare situations it happens.

It's obvious to me that the assignment always happens after evaluating
the expression on the RHS.  No optimizer should ignore the side effects
evaluating the expression might have.  Also keep in mind that the "++p"
could be in a preprocessor macro and be unnoticable for the programmer
(esp. if someone changes the header file after you wrote the code!).

If the optimizer breaks this I would say the optimizer is broken.  If I
have to read the details of the C standard to find something isn't
guaranteed to work, while the code looks fine to the average programmer,
then the C standard is broken, it doesn't do what "should happen".
Fortunately most compiler builders know this and define the undefined
behavior in the way most programmers expect it to work.

--
hundred-and-one symptoms of being an internet addict:
245. You use Real Audio to listen to a radio station from a distant
     city rather than turn on your stereo system.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Bram Moolenaar
In reply to this post by Matthew Winn

Matthew Winn wrote:

> On Tue, Jul 05, 2005 at 11:29:51AM +0200, Bram Moolenaar wrote:
> >
> > James Widman wrote:
> >
> > > Anyway: (i = ++i) seems, at first glance, like a safe (albeit  
> > > questionable) expression.  One might think (as I did) that,  
> > > regardless of which side of '=' is evaluated first,  the result is  
> > > the same.  But ISO C99, section 6.5,p2 says:
> > >
> > > "Between the previous and next sequence point an object shall have  
> > > its stored value modified at most once by the evaluation of an  
> > > expression."
> > >
> > > So according to C99, (p = vim_strchr(++p, '/')) results in undefined  
> > > behavior because p is modified twice.
> > >
> > > ISO C++ has basically the same verbiage.  I don't have our copies of  
> > > ISO C90 or the first-edition K&R C book nearby, so I don't yet know  
> > > the situation there.
> >
> > The problem here is the standard.  C99 is probably the worst C standard
> > that was ever made.  It was clearly formed by politicians, not by
> > practical programmers.  OK, well, that's just my opinion (a ran into a
> > couple of annoying problems).
> >
> > That "undefined behavior" is stupid.  Of course every good-old C
> > programmer can predict what should and will happen.  Only very bad C
> > compilers would not be able to deal with it.  The standard should be
> > adjusted to meet what programmers do and expect, not the other way
> > around.
>
> It's undefined so the compiler can do it as efficiently as possible.
> On an architecture where transferring information from the stack to
> memory is fast, an expression like q = func(++p) may be fastest if the
> incremented value of p isn't written back to the correct memory location
> until after q has been altered.

Well, the compiler can see whether the resulting value of p is used or
not, thus it can figure out what to do.

> Why should either speed of execution
> or speed of compilation suffer just to deal with the special case where
> a programmer attempts to alter the same value two different ways in
> one expression, especially as p = func(p+1) is almost certainly more
> efficient than p = func(++p) anyway?

In my opinion the "easy of use" of the language is much, much more
important than the easy to write a compiler for it.  After all,
programming languages are there to form the interface between a human
and a computer.  We can tell the computer to do whatever we want it to
do, that is quite a bit more difficult for humans :-).

It's possible for the compiler (or optimizer) to see the side effect and
act accordingly.  If there is no side effect, then use the optimal
implementation for speed.  If there is a side effect then make sure it
works as the programmer expects.

This does make the compiler (optimizer) a bit more complex, but that's
an order of magnitude less important than having the teach the human to
avoid this construct.  It's something you solve once and enjoy forever.

--
hundred-and-one symptoms of being an internet addict:
246. You use up your free 100 hours in less than a week.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Mike Williams
In reply to this post by Bram Moolenaar
Bram Moolenaar did utter on 05/07/2005 15:57:

> I'm glad I don't have to do that job interview :-).
>
> Of course it's not clean programming to change a pointer twice, but in
> rare situations it happens.
>
> It's obvious to me that the assignment always happens after evaluating
> the expression on the RHS.

Why do you say obvious?  By what definition?  In C, assignment is no
different to any other operator in an expression, which is why = is
called the assigment operator.  Assignment is not a statement like in
BASIC, Pascal, or Python, which is why you can do assignments within if,
while, etc. expressions.  Being an expression, the order of evaluation
of the left and right hand sides of the assignment operator can be done
in any order as per any other operator's operands.  All that the
standard specifies is that all side effects are resolved by the next
sequence point, nothing is said about the order of resolution between
sequence points.

 > No optimizer should ignore the side effects
> evaluating the expression might have.  

In C the term "side effect" has a defined meaning, which is essentially
modification of the execution state, i.e. the values of variables in
registers and memory.  So the simple expression i = 1; contains "side
effects" due to setting the variable i to the value 1.  "Side effects"
does not mean additional changes to the ones intended as possibly
understood with the ++/-- operators.

 > Also keep in mind that the "++p"
> could be in a preprocessor macro and be unnoticable for the programmer
> (esp. if someone changes the header file after you wrote the code!).

Indeed, and another typical C interview question where an argument to a
macro is something like "p++" and the argument appears twice in the body
of the macro leading to undefined behaviour (the increment may happen
once or twice).  Such macros should be written function like and macro
args assigned to block local variables to ensure the argument is
evaluated only once.

> If the optimizer breaks this I would say the optimizer is broken.  If I
> have to read the details of the C standard to find something isn't
> guaranteed to work, while the code looks fine to the average programmer,
> then the C standard is broken, it doesn't do what "should happen".

The benefit of C's approach is that the optimizer is simpler and
therefore less bug prone.  It is also more flexible across many
processor architectures providing efficient operation (smaller faster
binaries).  Remember that C is used with more embedded processors than
PCs where saving bytes and cycles multiplies into big money quickly.

Some might say that a lot of the issues with copmuters is because
average programmers don't actually understand the tools they use.  And C
is one of the simpler programming languages out there.

> Fortunately most compiler builders know this and define the undefined
> behavior in the way most programmers expect it to work.

I think you more mean that the few compilers on a couple of processor
architectures you are aware of happen to work in the way the code was
intended to work.  I haven't seen different compilers for the same
processor behave differently, but I have seen the issue appear when
porting to other processors.  Don't underestimate the benefit of jobs
running 10% faster due to optimisations possible from this flexibility in C.

Ok, this has gone way off topic.  Normal service (and lurking) will be
resumed. ;-)

TTFN

Mike
--
The decision is maybe and that's final.
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Matthew Winn
In reply to this post by Bram Moolenaar
On Tue, Jul 05, 2005 at 06:15:19PM +0200, Bram Moolenaar wrote:

>
> Matthew Winn wrote:
> > Why should either speed of execution
> > or speed of compilation suffer just to deal with the special case where
> > a programmer attempts to alter the same value two different ways in
> > one expression, especially as p = func(p+1) is almost certainly more
> > efficient than p = func(++p) anyway?
>
> In my opinion the "easy of use" of the language is much, much more
> important than the easy to write a compiler for it.  After all,
> programming languages are there to form the interface between a human
> and a computer.  We can tell the computer to do whatever we want it to
> do, that is quite a bit more difficult for humans :-).

What does p = func(++p) _mean_?  You're assigning two different values
to p at the same time: you're trying to give p the value p+1 and the
return value of func().  It's like saying int i = 1 = 2 and expecting
it to do both.  If you mean p = func(p+1) then say p = func(p+1): it'll
execute faster and it's easier for readers to understand.

In the case of expressions like i = ++i both assignments to i have the
same value, so it's easy to claim that it's obvious what should happen,
but what about less obvious cases?  What should
    int i = 1;
    i = ++i + ++i;
mean?  I can think of ways in which that could give any integer result
from 2 to 6 depending on the order in which registers are allocated and
the way values are incremented and stored.  It's easy to say that the
compiler should work out what was intended, but if humans can't decide
what was intended how can a compiler written by humans do it?

It's poor coding practice not only because the compiler's behaviour is
undefined, but also because anyone reading the source has to work harder
to determine the intention of the code.  If I saw
    p = vim_strchr(++p, '/');
I wouldn't know whether the "++p" was a mistake and should be "p+1", or
whether the "p =" was a mistake and the value was being assigned to the
wrong variable, or even whether the "++p" was a mistake and the wrong
variable was being incremented.  As written that code makes no sense,
and if I can't work out what it means the compiler certainly shouldn't.

--
Matthew Winn ([hidden email])
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Bram Moolenaar

Matthew Winn wrote:

> What does p = func(++p) _mean_?  You're assigning two different values
> to p at the same time: you're trying to give p the value p+1 and the
> return value of func().  It's like saying int i = 1 = 2 and expecting
> it to do both.  If you mean p = func(p+1) then say p = func(p+1): it'll
> execute faster and it's easier for readers to understand.

Order of evaluation for "=" is right to left.  I expect the expression
on the right to be evaluated before the result is assigned to the lvalue
on the left.  Isn't that what everybody expects?  You can't assign a
value before you know what the value is...

Another example where order of evaluation matters:

        n = (getc(fd) << 8) + getc(fd);

The expression with "+" is evaluated from left to right, thus the first
read byte is shifted.  This is not unusual code, right?

Adding up three byte values and putting the result in the fourth byte:

        *p = *p++ + *p++ + *p++;

It's weird, but I don't think that it is unclear about what is expected
to happen.

> In the case of expressions like i = ++i both assignments to i have the
> same value, so it's easy to claim that it's obvious what should happen,
> but what about less obvious cases?  What should
>     int i = 1;
>     i = ++i + ++i;
> mean?  I can think of ways in which that could give any integer result
> from 2 to 6 depending on the order in which registers are allocated and
> the way values are incremented and stored.  It's easy to say that the
> compiler should work out what was intended, but if humans can't decide
> what was intended how can a compiler written by humans do it?

If you know that evaluating the expression is done before the assignment
then it's clear what should happen.  But your example is unrealistic,
there is no point in doing the second increment.

Order of evaluation matters in many cases, thus it's not at all strange
to define this for the assignment.  Most notably for C is:

        if (p != NULL && *p == 1)

The && operator involves evaluating from left to right, and once it's
clear the result is False the evaluation stops.  This is defined in the
standard, right?  Otherwise a lot of code would break...

> It's poor coding practice not only because the compiler's behaviour is
> undefined, but also because anyone reading the source has to work harder
> to determine the intention of the code.  If I saw
>     p = vim_strchr(++p, '/');
> I wouldn't know whether the "++p" was a mistake and should be "p+1", or
> whether the "p =" was a mistake and the value was being assigned to the
> wrong variable, or even whether the "++p" was a mistake and the wrong
> variable was being incremented.  As written that code makes no sense,
> and if I can't work out what it means the compiler certainly shouldn't.

Right, the ++p doesn't make sense here, since the result of the
increment doesn't need to be stored.  The code isn't written like that,
the increment is done in a macro.  The code did make sense before
expanding the macro.  The macro might be suspicious, but isn't using
macros anyway?

--
Vi is clearly superior to emacs, since "vi" has only two characters
(and two keystrokes), while "emacs" has five.  (Randy C. Ford)

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Mike Williams
[ I feel there is much spleen venting going on here ;-) ]

Bram Moolenaar did utter on 06/07/2005 10:40:
> Matthew Winn wrote:
>
>>What does p = func(++p) _mean_?  You're assigning two different values
>>to p at the same time: you're trying to give p the value p+1 and the
>>return value of func().  It's like saying int i = 1 = 2 and expecting
>>it to do both.  If you mean p = func(p+1) then say p = func(p+1): it'll
>>execute faster and it's easier for readers to understand.
>
> Order of evaluation for "=" is right to left.

No it is not.  _Association_ for "=" is right to left, but the order of
evaluation of the operands of the "=" operator is undefined.  Given the
expresion a[i] = 2*b; a C compiler is allowed to calculate the memory
address for a[i] before evaluating the value 2*b, but doesn't have to.

 > I expect the expression
> on the right to be evaluated before the result is assigned to the lvalue
> on the left.  Isn't that what everybody expects?

They may expect it but they would be wrong.  Consider the expression
a[i++] = i; with i being 0 - what would you expect, and why?  And of
course you will be wrong ;-)

 >  You can't assign a
> value before you know what the value is...

The issue is that the expression of the object you assign the value to
has to be evaluated as well as the expression that generates the value
being assigned.  It is the order of these evaluations that is undefined.

> Another example where order of evaluation matters:
>
> n = (getc(fd) << 8) + getc(fd);
>
> The expression with "+" is evaluated from left to right, thus the first
> read byte is shifted.  This is not unusual code, right?
>
> Adding up three byte values and putting the result in the fourth byte:
>
> *p = *p++ + *p++ + *p++;
>
> It's weird, but I don't think that it is unclear about what is expected
> to happen.

That may be so but in both cases it is potentially buggy code.  If you
write code for a single platform/processor and it works for you then you
may be happy leaving it as it is, but if you want to write portable
code, including for new processors/platforms yet to appear, it should be
corrected.

>>In the case of expressions like i = ++i both assignments to i have the
>>same value, so it's easy to claim that it's obvious what should happen,
>>but what about less obvious cases?  What should
>>    int i = 1;
>>    i = ++i + ++i;
>>mean?  I can think of ways in which that could give any integer result
>>from 2 to 6 depending on the order in which registers are allocated and
>>the way values are incremented and stored.  It's easy to say that the
>>compiler should work out what was intended, but if humans can't decide
>>what was intended how can a compiler written by humans do it?
>
>
> If you know that evaluating the expression is done before the assignment
> then it's clear what should happen.  But your example is unrealistic,
> there is no point in doing the second increment.
>
> Order of evaluation matters in many cases, thus it's not at all strange
> to define this for the assignment.

Evaluation is defined in terms of "sequence points" not operators.  A
sequence point is the point in execution at which all side effects from
an evluation prior to point are complete and no side effects have
started for an evaluation after the point.

 > Most notably for C is:
>
> if (p != NULL && *p == 1)
>
> The && operator involves evaluating from left to right, and once it's
> clear the result is False the evaluation stops.  This is defined in the
> standard, right?  Otherwise a lot of code would break...

The logical operators && and || are special cases since they allow a
speed optimisation in conditional expressions.  There are programming
languages where the complete conditional expression is evaluated, which
would result in the above example causing a crash.  Who is to say which
is the natural way?  This can be very confusing to those new to
short-circuiting conditional expressions.

>>It's poor coding practice not only because the compiler's behaviour is
>>undefined, but also because anyone reading the source has to work harder
>>to determine the intention of the code.  If I saw
>>    p = vim_strchr(++p, '/');
>>I wouldn't know whether the "++p" was a mistake and should be "p+1", or
>>whether the "p =" was a mistake and the value was being assigned to the
>>wrong variable, or even whether the "++p" was a mistake and the wrong
>>variable was being incremented.  As written that code makes no sense,
>>and if I can't work out what it means the compiler certainly shouldn't.
>
> Right, the ++p doesn't make sense here, since the result of the
> increment doesn't need to be stored.  The code isn't written like that,
> the increment is done in a macro.  The code did make sense before
> expanding the macro.  The macro might be suspicious, but isn't using
> macros anyway?

TTFN

Mike
--
There are two ways to handle women. I don't know either one.
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Walter Briscoe
In reply to this post by Bram Moolenaar
In message <[hidden email]> of Wed, 6 Jul
2005 11:40:44 in , Bram Moolenaar <[hidden email]> writes

[snip]

>Right, the ++p doesn't make sense here, since the result of the
>increment doesn't need to be stored.  The code isn't written like that,
>the increment is done in a macro.  The code did make sense before
>expanding the macro.  The macro might be suspicious, but isn't using
>macros anyway?
>

Bram,
I think you mis-remember.
We have farsi.c(1954):   while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
I have main.i(316897):   while (((p = vim_strchr(++p, '/')) != ((void *)0)) && p[-1] == '\\')

I don't think there is anything unsafe about the code.
There is a sequence point before the call of vim_strchr.
There is another sequence point at && which means that p p[-1] refers to
the result of the assignment rather than the result of the increment.
Appendix C on sequence points in the C99 standard is helpful.

My proposed fix looks like c**p to me now. Do you agree this is clearer:
while (((p = vim_strchr(p+1, '/')) != NULL) && p[-1] == '\\')

As I said, I think PC-lint's sequence point model is adequate given its
purpose. This is the only vim thing [which] the simpler model diagnoses.
I insert that [which] to clarify my meaning. It is not strictly needed.
I think it improves my words. I first used "flags" where I now use
"diagnoses". It is sometimes hard to find an English verb which is not
also a noun and, so, requires more complicated syntax analysis.

Here, those whose first language is not English, do brilliantly, IMHO.
;)
--
Walter Briscoe
Reply | Threaded
Open this post in threaded view
|

Re: Mainly noise

Bram Moolenaar

Walter Briscoe wrote:

> In message <[hidden email]> of Wed, 6 Jul
> 2005 11:40:44 in , Bram Moolenaar <[hidden email]> writes
>
> [snip]
>
> >Right, the ++p doesn't make sense here, since the result of the
> >increment doesn't need to be stored.  The code isn't written like that,
> >the increment is done in a macro.  The code did make sense before
> >expanding the macro.  The macro might be suspicious, but isn't using
> >macros anyway?
> >
>
> Bram,
> I think you mis-remember.
> We have farsi.c(1954):   while (((p = vim_strchr(++p, '/')) != NULL) && p[-1] == '\\')
> I have main.i(316897):   while (((p = vim_strchr(++p, '/')) != ((void *)0)) && p[-1] == '\\')

Right, this was in a previous version.

> I don't think there is anything unsafe about the code.
> There is a sequence point before the call of vim_strchr.
> There is another sequence point at && which means that p p[-1] refers to
> the result of the assignment rather than the result of the increment.
> Appendix C on sequence points in the C99 standard is helpful.
>
> My proposed fix looks like c**p to me now. Do you agree this is clearer:
> while (((p = vim_strchr(p+1, '/')) != NULL) && p[-1] == '\\')

Looking at the line again I think this is indeed better.  I still don't
understand Farsi though...

--
Did you hear about the new 3 million dollar West Virginia State Lottery?
The winner gets 3 dollars a year for a million years.

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\              Project leader for A-A-P -- http://www.A-A-P.org        ///
 \\\     Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html   ///