U++ forum: Welcome to the forum

Search on this site

Search in forums

Home » U++ Library support » U++ Library : Other (not classified elsewhere) » Various fixes to uppsrc

Show: Today's Messages :: Show Polls :: Message Navigator
E-mail to friend

Various fixes to uppsrc [message #31064]

Thu, 03 February 2011 13:27

chickenk
Messages: 171
Registered: May 2007
Location: Grenoble, France

Experienced Member

Hello,

the attached diff contains various fixes and enhancements, mostly unrelated, found by the clang static analyzer and when trying to use Waf to build U++. Here they are:

CtrlLib/Splitter.cpp : move some initialisations from the ctor body to its initialization list, and set chstyle to NULL initially
Core/Defs.h : make the BREAK address volatile to avoid optimizations
art/BlueBar/BlueBar.upp : depends on CtrlLib
plugin/png/... : make the path to the headers uniform, also avoids confusion with the system png headers
Painter/Painter.upp : Should depend directly on the same basis of system libs as CtrlCore. I believe this one should be reviewed seriously, I'm not sure about the exact needs.

Feel free to split that into redmine tasks and treat them independently...

Regards,
Lionel

Attachment: upp_various_fixes.diff
(Size: 2.87KB, Downloaded 354 times)

Report message to a moderator

Re: Various fixes to uppsrc [message #31085 is a reply to message #31064]

Fri, 04 February 2011 10:43

chickenk
Messages: 171
Registered: May 2007
Location: Grenoble, France

Experienced Member

Another question: can someone explain the line below (CtrlLib/DocEdit.cpp line 325) and try to make it less ambiguous with parentheses ?

if(q >= 0 && q != cursor && delta < 0 == q < cursor && GetCaret(q).y != yy) {...}

We'll see if some opinions diverge about what this code generates... I suppose this is the following :

if(
      (q >= 0) &&
      (q != cursor) &&
      ( (delta < 0) == (q < cursor) ) &&
      (GetCaret(q).y != yy)
   ) {...}

[Updated on: Fri, 04 February 2011 10:51]

Report message to a moderator

Re: Various fixes to uppsrc [message #31093 is a reply to message #31085]

Fri, 04 February 2011 14:55

mirek
Messages: 14255
Registered: November 2005

Ultimate Member

chickenk wrote on Fri, 04 February 2011 04:43

Another question: can someone explain the line below (CtrlLib/DocEdit.cpp line 325) and try to make it less ambiguous with parentheses ?

if(q >= 0 && q != cursor && delta < 0 == q < cursor && GetCaret(q).y != yy) {...}

We'll see if some opinions diverge about what this code generates... I suppose this is the following :

if(
      (q >= 0) &&
      (q != cursor) &&
      ( (delta < 0) == (q < cursor) ) &&
      (GetCaret(q).y != yy)
   ) {...}

Yes. I have remade it to

		if(q >= 0 && q != cursor && (delta < 0) == (q < cursor) && GetCaret(q).y != yy) {

Mirek

Report message to a moderator

Re: Various fixes to uppsrc [message #31094 is a reply to message #31064]

Fri, 04 February 2011 15:07

mirek
Messages: 14255
Registered: November 2005

Ultimate Member

chickenk wrote on Thu, 03 February 2011 07:27

[*] CtrlLib/Splitter.cpp : move some initialisations from the ctor body to its initialization list, and set chstyle to NULL initially

Is this supposed to fix anything? (except maybe chstyle).

Quote:

[*] Core/Defs.h : make the BREAK address volatile to avoid optimizations

[*] art/BlueBar/BlueBar.upp : depends on CtrlLib

OK, applied.

Quote:

[*] plugin/png/... : make the path to the headers uniform, also avoids confusion with the system png headers

Wrong. If you would want to put the longer path there, you should have '<' instead of '"'.

Quote:

[*] Painter/Painter.upp : Should depend directly on the same basis of system libs as CtrlCore. I believe this one should be reviewed seriously, I'm not sure about the exact needs.[/list]

That would be a dire mistake. We want Painter to work even without X11.

However, I guess it SHOULD add fontconfig and freetype.

Mirek

[Updated on: Fri, 04 February 2011 15:11]

Report message to a moderator

Re: Various fixes to uppsrc [message #31096 is a reply to message #31094]

Fri, 04 February 2011 15:43

chickenk
Messages: 171
Registered: May 2007
Location: Grenoble, France

Experienced Member

mirek wrote on Fri, 04 February 2011 15:07

chickenk wrote on Thu, 03 February 2011 07:27

[*] CtrlLib/Splitter.cpp : move some initialisations from the ctor body to its initialization list, and set chstyle to NULL initially

Is this supposed to fix anything? (except maybe chstyle).

Indeed, does not fix anything apart from chstyle.

Quote:

[*] plugin/png/... : make the path to the headers uniform, also avoids confusion with the system png headers

Wrong. If you would want to put the longer path there, you should have '<' instead of '"'.

So in that case I believe we should have <plugin/png/png.h>. Do you agree?

Quote:

[*] Painter/Painter.upp : Should depend directly on the same basis of system libs as CtrlCore. I believe this one should be reviewed seriously, I'm not sure about the exact needs.[/list]

That would be a dire mistake. We want Painter to work even without X11.

However, I guess it SHOULD add fontconfig and freetype.

Mirek

Ouch, you're right. I can't remember exactly but I suppose I added these for fontconfig and freetype, but I added the X11 bits by mistake as well.

Thanks for looking at these Smile

Lionel

Report message to a moderator

Re: Various fixes to uppsrc [message #31097 is a reply to message #31094]

Fri, 04 February 2011 15:49

mirek
Messages: 14255
Registered: November 2005

Ultimate Member

mirek wrote on Fri, 04 February 2011 09:07

That would be a dire mistake. We want Painter to work even without X11.

However, I guess it SHOULD add fontconfig and freetype.

Mirek

However, those are included via Draw...

[Updated on: Fri, 04 February 2011 15:49]

Report message to a moderator

Re: Various fixes to uppsrc [message #31098 is a reply to message #31097]

Fri, 04 February 2011 15:50

chickenk
Messages: 171
Registered: May 2007
Location: Grenoble, France

Experienced Member

mirek wrote on Fri, 04 February 2011 15:49

mirek wrote on Fri, 04 February 2011 09:07

That would be a dire mistake. We want Painter to work even without X11.

However, I guess it SHOULD add fontconfig and freetype.

Mirek

However, those are included via Draw...

So maybe the fix is to make Painter depend on Draw instead of Core like it is currently ?

Report message to a moderator

Re: Various fixes to uppsrc [message #31099 is a reply to message #31096]

Fri, 04 February 2011 15:51

mirek
Messages: 14255
Registered: November 2005

Ultimate Member

chickenk wrote on Fri, 04 February 2011 09:43

mirek wrote on Fri, 04 February 2011 15:07

chickenk wrote on Thu, 03 February 2011 07:27

[*] CtrlLib/Splitter.cpp : move some initialisations from the ctor body to its initialization list, and set chstyle to NULL initially

Is this supposed to fix anything? (except maybe chstyle).

Indeed, does not fix anything apart from chstyle.

Quote:

[*] plugin/png/... : make the path to the headers uniform, also avoids confusion with the system png headers

Wrong. If you would want to put the longer path there, you should have '<' instead of '"'.

So in that case I believe we should have <plugin/png/png.h>. Do you agree?

Well, maybe, however #include ".." is AFAIK meant exactly to tell compiler to take 'local' header file (in the same dir) and it is used in this context U++-wide...

Mirek

Report message to a moderator

Re: Various fixes to uppsrc [message #31100 is a reply to message #31099]

Fri, 04 February 2011 15:53

chickenk
Messages: 171
Registered: May 2007
Location: Grenoble, France

Experienced Member

mirek wrote on Fri, 04 February 2011 15:51

chickenk wrote on Fri, 04 February 2011 09:43

mirek wrote on Fri, 04 February 2011 15:07

chickenk wrote on Thu, 03 February 2011 07:27

[*] CtrlLib/Splitter.cpp : move some initialisations from the ctor body to its initialization list, and set chstyle to NULL initially

Is this supposed to fix anything? (except maybe chstyle).

Indeed, does not fix anything apart from chstyle.

Quote:

[*] plugin/png/... : make the path to the headers uniform, also avoids confusion with the system png headers

Wrong. If you would want to put the longer path there, you should have '<' instead of '"'.

So in that case I believe we should have <plugin/png/png.h>. Do you agree?

Well, maybe, however #include ".." is AFAIK meant exactly to tell compiler to take 'local' header file (in the same dir) and it is used in this context U++-wide...

Mirek

So leave it as-is, I agree with you. It must be an error from my build system at that time (it was some time ago). I will try to reproduce it and correct my build system instead.

Report message to a moderator

Re: Various fixes to uppsrc [message #31101 is a reply to message #31098]

Fri, 04 February 2011 16:06

mirek
Messages: 14255
Registered: November 2005

Ultimate Member

chickenk wrote on Fri, 04 February 2011 09:50

mirek wrote on Fri, 04 February 2011 15:49

mirek wrote on Fri, 04 February 2011 09:07

That would be a dire mistake. We want Painter to work even without X11.

However, I guess it SHOULD add fontconfig and freetype.

Mirek

However, those are included via Draw...

So maybe the fix is to make Painter depend on Draw instead of Core like it is currently ?

Right! Actually, I thought it already was...

Report message to a moderator

Previous Topic:	GCC warnings patch - mega mix 1 - unused variables and contructor var init order (SVN r2960)
Next Topic:	Issues with new stable release 3211

Goto Forum:

-=] Back to Top [=-

[ Syndicate this forum (XML) ] [

] [

]

Current Time: Sat Apr 26 19:33:23 CEST 2025

Total time taken to generate the page: 0.01006 seconds