View Issue Details

IDProjectCategoryView StatusLast Update
0002077SkyChart1-Softwarepublic19-02-09 11:12
ReporterSasa Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionreopened 
PlatformLinuxOSUbuntu 64-bitOS Version18.10
Product Version4.1 SVN 
Summary0002077: Catalog issues - AV loading unexisted file
DescriptionBasically, I have tested installation process for ubuntu:
https://www.ap-i.net/skychart/en/documentation/installation_on_linux_ubuntu

installed additional catalogs in step 5. After that I have set many catalogs to be used from CDC settings, not necessary available. In some moment, after several zoom in, attached message is shown when run from IDE.

Starting from console, nothing indicate about error, nor error is shown as file loading attempt is probably inside try finally end block.
TagsNo tags attached.

Activities

Sasa

19-02-07 05:29

reporter  

Sasa

19-02-07 05:38

reporter   ~0005349

Last edited: 19-02-07 05:38

In addition, this is skychart.ini. With this ini file, error is always present after several zoom in steps running from IDE.

skychart.zip (7,504 bytes)

Patrick Chevalley

19-02-07 17:25

administrator   ~0005354

This is a problem when you run in debug from the IDE, it show every exception, even normal exception that are trapped in a try...except.

This error is raised from line 872 of u_290.pas, there is a except block that disable further access in case of exception. We cannot check in advance for the presence and validity of every catalog file, doing this way is probably the safer.
So if you run the program normally without gdb you not see anything.

If the directory is not present at all the path is draw in red in the catalog setup, this must be sufficient to tell you something is wrong.

Sasa

19-02-09 09:13

reporter   ~0005369

This AV is really annoying during debugging. Following code is a bit more friendly:

         (*
         try
           thefile_stars:=tfilestream.Create( catalog_path+name_star, fmOpenRead );
           Reader_stars := TReader.Create (thefile_stars, 5*6*9*11);{number of hnsky records, multiply off all posible record sizes}
           {thefile_stars.size-reader.position>sizeof(hkyhdr) could also be used but slow down a factor of 2 !!!}
           files_available:=true;
         except
            readdatabase290:=false;
            files_available:=false;
            exit;
         end;
         *)

         if FileExists(catalog_path+name_star) then
         begin

           try
             thefile_stars:=tfilestream.Create( catalog_path+name_star, fmOpenRead );
             Reader_stars := TReader.Create (thefile_stars, 5*6*9*11);{number of hnsky records, multiply off all posible record sizes}
             {thefile_stars.size-reader.position>sizeof(hkyhdr) could also be used but slow down a factor of 2 !!!}
             files_available:=true;
           except
            readdatabase290:=false;
            files_available:=false;
            exit;
           end

         end
         else
         begin
           readdatabase290:=false;
           files_available:=false;
           exit;
         end;

I do not know if TReader.Create may rise some exception, thus I left all code inside try-except-end block. Otherwise, can be removed.

Sasa

19-02-09 09:46

reporter   ~0005370

And, BTW, I do not see anywhere related thefile_stars.Free or similar.

Patrick Chevalley

19-02-09 09:58

administrator   ~0005372

Why do you not simply remove the checkbox for this catalog? no more AV in debugging.
It is in the tab "other software data", uncheck "HNSKY type 290 database"

I not want to change this code because it is imported from HNSKY with minimal change to ease the maintenance if Han make change in the file format.

Sasa

19-02-09 10:56

reporter   ~0005374

Last edited: 19-02-09 10:58

Of course I can uncheck, but it is enabled in advance and I really do not need to know anything about since I simply want to test other functionality have nothing relating with.

It is simply not correct way how file loading it handled. Try except is used for unexpected events only. And since we know that if file do not exist the instance class will raise AV, we can handle that in advance. Anyway, that is my practice always.

Not freeing it correctly left memory leaks.

Patrick Chevalley

19-02-09 11:12

administrator   ~0005375

OK, the error is the catalog is enabled by default. Probably left over during my testing.
this is fixed:
https://github.com/pchev/skychart/commit/577e031ecbfd98970a7dfb6a210be8b243ca4bf8

Issue History

Date Modified Username Field Change
19-02-07 05:29 Sasa New Issue
19-02-07 05:29 Sasa File Added: Screenshot from 2019-02-07 05-02-00.png
19-02-07 05:38 Sasa File Added: skychart.zip
19-02-07 05:38 Sasa Note Added: 0005349
19-02-07 05:38 Sasa Note Edited: 0005349
19-02-07 17:25 Patrick Chevalley Status new => resolved
19-02-07 17:25 Patrick Chevalley Resolution open => no change required
19-02-07 17:25 Patrick Chevalley Note Added: 0005354
19-02-09 09:13 Sasa Status resolved => new
19-02-09 09:13 Sasa Resolution no change required => reopened
19-02-09 09:13 Sasa Note Added: 0005369
19-02-09 09:46 Sasa Note Added: 0005370
19-02-09 09:58 Patrick Chevalley Note Added: 0005372
19-02-09 10:56 Sasa Note Added: 0005374
19-02-09 10:57 Sasa Note Edited: 0005374
19-02-09 10:57 Sasa Note Edited: 0005374
19-02-09 10:58 Sasa Note Edited: 0005374
19-02-09 10:58 Sasa Note Edited: 0005374
19-02-09 11:12 Patrick Chevalley Note Added: 0005375