Jump to content


Photo

merge requests for PLi's git


  • Please log in to reply
1748 replies to this topic

Re: merge requests for PLi's git #581 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 1 August 2014 - 19:43

fixed file handle leak on read error in 'lib/service/servicedvd.cpp'

Attached Files


Edited by captnoord, 1 August 2014 - 19:44.


Re: merge requests for PLi's git #582 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 1 August 2014 - 19:53

Fixed while loop scope not being respected in 'lib/gui/elistbox.cpp'

 

This one is a wierd one, because currently 'something' gets executed.

situation could be compared to:

 

int number = 10;

while (--number);

{

    do_function(number); // is only called once, after the while has completed.

}

 

The guy who wrote that, could have meant:

 

int number = 10;

while (--number) // <--- ';' removed.

{

    do_function(number); // its called 10x

}

 

 

So that's what I did, currently can't test the change as my girl is watching tv :P

Attached Files



Re: merge requests for PLi's git #583 littlesat

  • PLi® Core member
  • 57,062 posts

+698
Excellent

Posted 1 August 2014 - 20:41

Thanks.... I suppose Erik will pick it up.... ;)


WaveFrontier 28.2E | 23.5E | 19.2E | 16E | 13E | 10/9E | 7E | 5E | 1W | 4/5W | 15W


Re: merge requests for PLi's git #584 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 1 August 2014 - 20:56

I really, really appreciate to see someone cleaning up.

 

Your welcome



Re: merge requests for PLi's git #585 mirakels

  • Forum Moderator
    PLi® Core member
  • 7,603 posts

+62
Good

Posted 1 August 2014 - 22:06

fixed file handle leak on read error in 'lib/service/servicedvd.cpp'

The code bcomes a bit more 'ugly' this way.

Such code warantsthe use of goto's, but in this case code could be simplified by using if (fread() && fread() && fread() && fread()) { ...}

 

I wonder why  'what = nothl(what);'  is done twice  (already in the original code)


Geen wonder... Had slechts een dm7000, maar wel ook een rotor. eigenlijk al een tijdje ook een dm600 en dm7025. Maar nu kijkend met een et9000 en vuduo

Re: merge requests for PLi's git #586 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 1 August 2014 - 22:14

I agree, I could take some more time and actually do a bit of re-factoring.



Re: merge requests for PLi's git #587 mirakels

  • Forum Moderator
    PLi® Core member
  • 7,603 posts

+62
Good

Posted 1 August 2014 - 22:19

Fixed while loop scope not being respected in 'lib/gui/elistbox.cpp'

 

This one is a wierd one, because currently 'something' gets executed.

situation could be compared to:

 

int number = 10;

while (--number);

{

    do_function(number); // is only called once, after the while has completed.

}

 

The guy who wrote that, could have meant:

 

int number = 10;

while (--number) // <--- ';' removed.

{

    do_function(number); // its called 10x

}

 

 

So that's what I did, currently can't test the change as my girl is watching tv :P

Good catch captnoord. I introduced this 'bug  almost 2 and a half years ago! apparently this border case almost never happens.

I thing the patch is correct!


Geen wonder... Had slechts een dm7000, maar wel ook een rotor. eigenlijk al een tijdje ook een dm600 en dm7025. Maar nu kijkend met een et9000 en vuduo

Re: merge requests for PLi's git #588 mirakels

  • Forum Moderator
    PLi® Core member
  • 7,603 posts

+62
Good

Posted 1 August 2014 - 22:30

applied the elistbox patch from captnoord.

How did you create the patch? 'git am' refused to apply it because of patch format error. Did you use 'git format-patch'?

 

(so applied it manually this time.)


Geen wonder... Had slechts een dm7000, maar wel ook een rotor. eigenlijk al een tijdje ook een dm600 en dm7025. Maar nu kijkend met een et9000 en vuduo

Re: merge requests for PLi's git #589 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 1 August 2014 - 22:37

i'm a lazy gitk user and used the 'write commit to file' option. Next time I will create them using git format-patch.



Re: merge requests for PLi's git #590 littlesat

  • PLi® Core member
  • 57,062 posts

+698
Excellent

Posted 1 August 2014 - 22:43

Again thanks!

WaveFrontier 28.2E | 23.5E | 19.2E | 16E | 13E | 10/9E | 7E | 5E | 1W | 4/5W | 15W


Re: merge requests for PLi's git #591 MiLo

  • PLi® Core member
  • 14,055 posts

+298
Excellent

Posted 2 August 2014 - 09:15

fixed file handle leak on read error in 'lib/service/servicedvd.cpp'

There is a CFile wrapper that prevents this kind of kludgy code. It's compatible with FILE* but since the destructor cleans up, you don't have to add all those "close" calls.
Real musicians never die - they just decompose

Re: merge requests for PLi's git #592 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 2 August 2014 - 11:38

fixed file handle leak on read error in 'lib/service/servicedvd.cpp'

There is a CFile wrapper that prevents this kind of kludgy code. It's compatible with FILE* but since the destructor cleans up, you don't have to add all those "close" calls.

Thats even better, but if we are doing that we should just search the code for any other use of FILE and replace it with CFile where possible.
Incremental improvement ftw.



Re: merge requests for PLi's git #593 MiLo

  • PLi® Core member
  • 14,055 posts

+298
Excellent

Posted 2 August 2014 - 20:08

Thats even better, but if we are doing that we should just search the code for any other use of FILE and replace it with CFile where possible.

+1
Real musicians never die - they just decompose

Re: merge requests for PLi's git #594 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 2 August 2014 - 20:43

Thats even better, but if we are doing that we should just search the code for any other use of FILE and replace it with CFile where possible.

+1

I have looked into the matter a bit, I personally don't the CFile 'struct' very complete. I'm extending the CFile class
so that it fits the needs of what I have in mind.

Don't worry about patches for now as it is a little more work than I was hoping for.



Re: merge requests for PLi's git #595 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 2 August 2014 - 20:53

Btw, what is the lowest compiler version I should worry about?



Re: merge requests for PLi's git #596 MiLo

  • PLi® Core member
  • 14,055 posts

+298
Excellent

Posted 3 August 2014 - 14:50

I have looked into the matter a bit, I personally don't the CFile 'struct' very complete. I'm extending the CFile class
so that it fits the needs of what I have in mind.

Yeah, you could add read/write methods to it so it looks a bit more OO, so you can write this:

CFile f("/tmp/file", "r");
ssize_t bytes = ::fread(buffer, sizeof(buffer), 1, f);

like this:

CFile f("/tmp/file", "r");
ssize_t bytes = f.read(buffer, sizeof(buffer), 1);
Real musicians never die - they just decompose

Re: merge requests for PLi's git #597 MiLo

  • PLi® Core member
  • 14,055 posts

+298
Excellent

Posted 3 August 2014 - 14:53

Note that when you want to read binary files in larger chunks, you're much better of using "open()" and "read()" instead. FILE* is only good for parsing text files and processing files in tiny chunks.
Real musicians never die - they just decompose

Re: merge requests for PLi's git #598 captnoord

  • Member
  • 22 posts

+10
Neutral

Posted 4 August 2014 - 06:57

Don't worry, I know.

I was also thinking about something like this:

 

CFile f;

if (!f.open("/tmp/file", "r"))

   return false;


ssize_t bytes = f.read(buffer, sizeof(buffer), 1); 


Edited by captnoord, 4 August 2014 - 06:59.


Re: merge requests for PLi's git #599 MiLo

  • PLi® Core member
  • 14,055 posts

+298
Excellent

Posted 4 August 2014 - 09:29

Currently, you should use:
CFile f("/tmp/file", "r");
if (!f)
  return false;
I would have preferred:
try
{
  CFile f("/tmp/file", "r");
  f.read(buffer, sizeof(buffer));
  ...
}
catch (const std::exception& ex)
{
  report_error_somewhere(ex.what());
}
but some things in the E2 code fail to compile when C++ exceptions are enabled.

Edited by MiLo, 4 August 2014 - 09:30.

Real musicians never die - they just decompose

Re: merge requests for PLi's git #600 MiLo

  • PLi® Core member
  • 14,055 posts

+298
Excellent

Posted 4 August 2014 - 09:32

If you add an "open" method, it will need a "close" method as well, and "open" must (counterintuitively) call "close" too.

I don't see the added value of that, you might as well create a new object for a new file.
Real musicians never die - they just decompose


23 user(s) are reading this topic

0 members, 23 guests, 0 anonymous users