Access violation TStringList

Trying to figure out why this doesn't work:

cdromList := GetCDROMDrives;


where GetCDROMDrives returns a TStringList;

There is some confusion on my part as to how to create, assing, and free a TStringList properly.


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


unit installpoint;

interface

uses
  Windows, Messages, SysUtils, Classes, Graphics, Controls, Forms, Dialogs,
  StdCtrls;

type
  TFormInstall = class(TForm)
    LabelInstallInstructions: TLabel;
    ButtonInstall: TButton;
    procedure ButtonInstallClick(Sender: TObject);
    function GetCDROMDrives: TStringList;
  private
    { Private declarations }
  public
    { Public declarations }
  end;

var
  FormInstall: TFormInstall;

implementation

{$R *.DFM}

procedure TFormInstall.ButtonInstallClick(Sender: TObject);
var
  cdromList : TStringList;
begin
    cdromList := TStringList.Create;

    cdromList.Clear;

    cdromList := GetCDROMDrives;

    ShowMessage(cdromList[0]);
end;

function TFormInstall.GetCDROMDrives: TStringList;
var
  DriveBits: set of 0..25;
  Drives,DriveNum: integer;
  DriveLetter: string;
  list : TStringList;
begin
  list := TStringList.Create;
  list.Clear;
  Drives := GetLogicalDrives;
  if Drives <> 0 then
  begin
    integer(DriveBits) := Drives;
    for DriveNum := 0 to 25 do
    begin
      if (DriveNum in DriveBits) then
      begin
        DriveLetter := char(DriveNum+Ord('A'))+':';
        if GetDriveType(PChar(DriveLetter)) = DRIVE_CDROM then
          list.Add(DriveLetter);
      end;
    end;
  end;

  GetCDROMDrives := list;

  list.Free;

end;

end.
LVL 5
Tom KnowltonWeb developerAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

geobulCommented:
Hi,

The following is a simple example how this could be done:

function GetSL:TStringList;
begin
  result := TStringList.Create; // create the StringList
  result.Add('aaa');
  result.Add('bbb');
  result.Add('ccc');
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  sl: TStringList;
begin
  sl := GetSL; // 'Create' is inside the function
  ComboBox1.Items := sl;
  sl.Free; // 'Free' is outside the function
end;

Regards, Geo
0
petervullingsCommented:
If you want the main procedure to manage the creation and freeing, try something like:

procedure TFormInstall.ButtonInstallClick(Sender: TObject);
var
 cdromList : TStringList;
begin
   cdromList := TStringList.Create;
   GetCDROMDrives(cdromList)
   ShowMessage(cdromList[0]);
end;

procedure TFormInstall.GetCDROMDrives( list : TStringList );
var
 DriveBits: set of 0..25;
 Drives,DriveNum: integer;
 DriveLetter: string;
 list : TStringList;
begin
 {check that list is created}
 if not assigned(list) then exit;
 list.Clear;
 Drives := GetLogicalDrives;
 if Drives <> 0 then
 begin
   integer(DriveBits) := Drives;
   for DriveNum := 0 to 25 do
   begin
     if (DriveNum in DriveBits) then
     begin
       DriveLetter := char(DriveNum+Ord('A'))+':';
       if GetDriveType(PChar(DriveLetter)) = DRIVE_CDROM then
         list.Add(DriveLetter);
     end;
   end;
 end;
end;

Thats it.

Points to remember are that when an object is passed between functions, you are actually passing the object itself - not a copy of it.  To understand this you need to realise that 'list' is actually pointer to the TStringList, but Delphi makes it so you dont really have to concern yourself with this.

With a string:

var
  str : string;
begin
  str := 'aaa';
  doSomething( str );
  showMessage(str); { will still show 'aaa' }
end;

procedure doSomething( str : string );
begin
  str := 'bbb';
end;

With an object (e.g String List )
var
  str : TStringList;
begin
  str := TStringList.create;
  str.add('aaa');
  doSomething( str );
  showMessage(str[0]); { will now show 'bbb' }
  str.free;
end;

procedure doSomething( str : TStringList);
begin
  str[0] := 'bbb';
end;
 
Hope it helps,
Pea
0
keashFCommented:
The reason why your code fails is the last line in getCDRomDrives:

list.free;

Ater passing the reference of list to getcdromdrive, you destroy the list and with it the result -> so you'll have a nil pointer as result.

check out this:

function test:TStringList;
var list:TStringList;
begin
list:=TStringList.create;
list.add('one');
list.add('two');
result:=list;
list.free;    // <-- this is the error
end;

procedure TForm1.Button1Click(Sender: TObject);
var l : TStringList;
begin
   l:=test;
   showmessage(l[0]);
end;

remove the errornous line and it will work, however i suggest you use petervullings approach.

cu
keashF
0
Get expert help—faster!

Need expert help—fast? Use the Help Bell for personalized assistance getting answers to your important questions.

petervullingsCommented:
If you do go with keashF's approach, remember to 'create' the string list only once - so if it is being created within the function 'getCDRomDrives', dont create it in the 'ButtonInstallClick' procedure. And, yes, don't 'free' it before you try to use it!
0
petervullingsCommented:
p.s. It is good practice to free the object in the same function that created it...
0
BorlandManCommented:

A simple way to make your code work here is to add the Stringlist as a param in your GetCDRoms and use the parameter in your GetCDROMDrives() work code. Once you exit that procedure , the method which called it will have the Stringlist with the correct values in it.

hth,
J


procedure TFormInstall.ButtonInstallClick(Sender: TObject);
var
 cdromList : TStringList;
begin
   cdromList := TStringList.Create;
   cdromList.Clear;

   GetCDROMDrives (cdRomList);
   ShowMessage(cdromList[0]);
end;

procedure TFormInstall.GetCDROMDrives(ACdRomList:  
                                   TStringList);
var
 DriveBits: set of 0..25;
 Drives,DriveNum: integer;
 DriveLetter: string;
begin
 list.Clear;
 Drives := GetLogicalDrives;
 if Drives <> 0 then
 begin
   integer(DriveBits) := Drives;
   for DriveNum := 0 to 25 do
   begin
     if (DriveNum in DriveBits) then
     begin
       DriveLetter := char(DriveNum+Ord('A'))+':';
       if GetDriveType(PChar(DriveLetter)) = DRIVE_CDROM then
         ACdRomList.Add(DriveLetter);
     end;
   end;
 end;

end;

end.
0
sfockCommented:
hi guys, good comments, but

petervullings

>when an object is passed between functions, you are actually passing the object itself - not a copy of it.
it's not the obect itself but a reference to the object itself

keashF
> you destroy the list and with it the result -> so you'll have a nil pointer as result.
the reference will not be nil it will still point to the obejects former address, but the object will be destructed
and
>remove the errornous line and it will work,
yes the exception will be gone but you produce memory leaks

BorlandMan
what is different in your solution to petervullings solution?

knowlton
i admit to use petervullings solution.

0
geobulCommented:
Hi,

You all will produce memory leak suggesting more or less:
--Quote
procedure TFormInstall.ButtonInstallClick(Sender: TObject);
var
cdromList : TStringList;
begin
  cdromList := TStringList.Create;
  GetCDROMDrives(cdromList)
  ShowMessage(cdromList[0]);
end;
--End of quote

Reason: cdromList is a local variable inside the procedure which runs out of scope when the procedure ends up its execution. Without freeing cdromList at the end of the function you've got a memory leak.

So, add cdromList.Free; at the end. Usage of try..finally is recommended:
...
cdromList := TStringList.Create;
try
  // do something with the list
finally
  cdromList.Free;
end;
...

I agree that passing a stringlist as a parameter is more clearer but in my example I followed the definitions in the question and I have no memory leak and no AV.

Regards, Geo
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Tom KnowltonWeb developerAuthor Commented:
Thank you everyone.
0
BorlandManCommented:
uh,
sorry I must've missed it.  I didn't mean to repeat an existing solution.... but was going to indicate that you need to free the stringlist, which I obviously forgot to do to - (yep here's an excuse) I was in a hurry to get it done, because It was late and had to pack for a holiday.. I guess that's what I get for trying to rush

sorry,
thanks,
J
0
joinedCommented:
You can try EurekaLog (www.eurekalog.com).

EurekaLog is an add-in tool that gives to your application (GUI, Console, Web, etc.) the ability to catch every exception, and generates a detailed log of call stack (with unit, class, method and line #), showing and sending it back to you via email.

--
Best regards...

Fabio Dell'Aria.
0
Tom KnowltonWeb developerAuthor Commented:
Thanks joined.  The issue was resolved satisfactorily some time ago, but I appreciate the info.

Tom
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Delphi

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.