Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
Home » U++ Library support » U++ Library : Other (not classified elsewhere) » Various fixes to uppsrc
Various fixes to uppsrc [message #31064] Thu, 03 February 2011 13:27 Go to next message
chickenk is currently offline  chickenk
Messages: 169
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:
  1. CtrlLib/Splitter.cpp : move some initialisations from the ctor body to its initialization list, and set chstyle to NULL initially
  2. Core/Defs.h : make the BREAK address volatile to avoid optimizations
  3. art/BlueBar/BlueBar.upp : depends on CtrlLib
  4. plugin/png/... : make the path to the headers uniform, also avoids confusion with the system png headers
  5. 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
Re: Various fixes to uppsrc [message #31085 is a reply to message #31064] Fri, 04 February 2011 10:43 Go to previous messageGo to next message
chickenk is currently offline  chickenk
Messages: 169
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 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13975
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
Re: Various fixes to uppsrc [message #31094 is a reply to message #31064] Fri, 04 February 2011 15:07 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13975
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 Go to previous messageGo to next message
chickenk is currently offline  chickenk
Messages: 169
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:

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:

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
Re: Various fixes to uppsrc [message #31097 is a reply to message #31094] Fri, 04 February 2011 15:49 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13975
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 Go to previous messageGo to next message
chickenk is currently offline  chickenk
Messages: 169
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 ?
Re: Various fixes to uppsrc [message #31099 is a reply to message #31096] Fri, 04 February 2011 15:51 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13975
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:

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
Re: Various fixes to uppsrc [message #31100 is a reply to message #31099] Fri, 04 February 2011 15:53 Go to previous messageGo to next message
chickenk is currently offline  chickenk
Messages: 169
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:

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.
Re: Various fixes to uppsrc [message #31101 is a reply to message #31098] Fri, 04 February 2011 16:06 Go to previous message
mirek is currently offline  mirek
Messages: 13975
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...
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:
  


Current Time: Thu Mar 28 15:11:05 CET 2024

Total time taken to generate the page: 0.01653 seconds