1

I am currently working on a program that needs to be able to report whether or not Windows has been activated.

function TSoftwareReport.getLicenceInfo : String;
var
  Activated : String;
  LicenceFile : TextFile;
begin
  // redirect command output to txt file
  ShellExecute(0,nil,'cmd.exe', '/C cscript %windir%\system32\slmgr.vbs/xpr > C:\Activated.txt', nil, SW_HIDE);
  //Read the file
  Sleep(1000);
  AssignFile(LicenceFile, 'C:\Activated.txt');
  Reset(LicenceFile);
  while not Eof(LicenceFile) do
  begin
    ReadLn(LicenceFile,Activated);
    if AnsiContainsText(Activated, 'Permanently') then
    begin
      Activated := 'Permanent';
      Break;
    end;
  end;
  //cleanup file
  CloseFile(LicenceFile);
  DelDir('C:\Activated.txt');
  Result := Activated;
end;

Currently I am using the ShellExecute and redirecting the output to a text file, that I then want to read in order to find out if Windows has been activated.

The problem I am having is that when I go to do the ShellExecute line followed by the AssignFile line I get an I/O 32 error. I suspect that this is due to ShellExecute not closing the file before I attempt to access it with "AssignFile". Which is why I now have the Sleep() line in. The problem with this is that I don't know how long I should be sleeping for as performance will be different across machines.

I tried writing code that would try running the AssignFile line and if that fails, then sleep() and try again but now I'm throwing exceptions on purpose. The whole thing just feels hacky and badly written. I already feel bad about having to redirect the output from shellexecute to a text file.

So the question is, Should I be using sleep, and if so how do I decide how long to sleep for? Should I be using an alternative?

Michael G
  • 13
  • 2

1 Answers1

3

Should I be using sleep?

No.

Should I be using an alternative?

Yes. Use a wait function to block until the process you created has terminated. Then read the output file.

There are a few options that will affect your design choices.

In order to wait, you need to obtain a process handle. You can do that with ShellExecuteEx or CreateProcess. But not with ShellExecute.

If you wish to redirect output, and don't want to code the redirection yourself, then you need to continue using cmd.exe and getting it to perform the redirection. Alternatively you can use CreateProcess and supply either a file handle or a pipe handle as the new process stdout handle.

If I were you I would avoid creating a file that is just going to be used for transient output. This is what pipes are made for. Create a pipe and feed its write end to the new process as the stdout of that new process. Read the contents out of the read end of the pipe. That way you avoid spewing files into your file system. What's more it allows you to avoid spawning a cmd.exe process that you just do not need.

One final point to make is that you don't check for errors when calling ShellExecute. Granted, ShellExecute doesn't report errors very well, but you simply must learn to check for errors when calling API functions. If the functions fail, and you don't check for errors, how can you expect to diagnose what went wrong?

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490