Watch, Follow, &
Connect with Us

For forums, blogs and more please visit our
Developer Tools Community.


Welcome, Guest
Guest Settings
Help

Thread: FileOpen Question



Permlink Replies: 18 - Last Post: Apr 24, 2017 2:20 PM Last Post By: William Macon Threads: [ Previous | Next ]
William Macon

Posts: 19
Registered: 2/24/16
FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 1, 2017 5:09 PM
I have been migrating old C++ code from CodeGear to Berlin and finally got a successful compile; thank you Remy. I am now stuck on the program crashing after opening (I do get the opening screen) because it's not opening a file that is there. Code snippet:

int load_project(char *filename)
{
int c = 0;
int f = 0;
int buff = 0;

//OPEN FILE
f = FileOpen(filename, fmOpenRead);
if (f == -1)
return -1;

So, "filename" is a const AnsiString FileName per the SysUtils.FileOpen syntax. I have tried changing char *filename to AnsiString filename but that causes an error. I tried changing it to char filename[255] but that had no effect. Why do I keep getting a return value of -1 for "f" that indicates that it could not obtain a file handle and an error occurred? A previous find_file function in the code works for the file in question, and the code continues to this next load_project function which does not work. 

I'd like to understand what's going on with these file operations now and what I need to do to resolve this and similar issues moving forward. Thanks.
Asger Joergensen

Posts: 370
Registered: 11/18/08
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 2, 2017 1:04 AM   in response to: William Macon in response to: William Macon
Hi William

The SysUtils help say:

int FileOpen(const UnicodeString FileName, LongWord Mode);

So your function should look like this:

int load_project( String filename )
{
   int c = 0;
   int f = 0;
   int buff = 0;
   //OPEN FILE
   f = FileOpen(filename, fmOpenRead);
   if (f == -1)
      return -1;
....


But nothing in the code you show will make your program crash, so you need to
set a break point on the FileOpen and go through your code using F8 to see
where it crash.

Best regards
Asger
William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 3, 2017 6:23 AM   in response to: Asger Joergensen in response to: Asger Joergensen
Asger Joergensen wrote:
The SysUtils help say:
int FileOpen(const UnicodeString FileName, LongWord Mode);

So your function should look like this:

int load_project( String filename )
{
   int c = 0;
   int f = 0;
   int buff = 0;
   //OPEN FILE
   f = FileOpen(filename, fmOpenRead);
   if (f == -1)
      return -1;
....


But nothing in the code you show will make your program crash, so you need to
set a break point on the FileOpen and go through your code using F8 to see
where it crash.

OK, thanks, I note that I was looking at an older version of the SysUtils help. I am still getting either compiler errors or code failure as I try different approaches with converting UnicodeString FileName, either how it's defined, or called or used in sprintf statements. Also, it's not that the code itself that is crashing but rather another conditional check that if it cannot find/open the necessary files that it closes.

I am continuing to read through the current SysUtils help and other references and try different things to get through this. I understand how to deal with old char strings to resolve the compiler errors, but the nuances of dealing with the old char filenames in the new Unisys functions looking for UnicodeString is confusing. For example, I tried:
f = FileOpen(AnsiString(filename), fmOpenRead);
and that didn't work. So I'm missing something fundamental regarding how to covert old char filenames to UnicodeString filenames, which is important for dealing with open/read/write functions throughout the code. Any additional suggestions or guidance would be helpful.

Edited by: William Macon on Apr 3, 2017 6:24 AM
Asger Joergensen

Posts: 370
Registered: 11/18/08
Re: FileOpen Question [Edit]
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 3, 2017 6:39 AM   in response to: William Macon in response to: William Macon
Hi William

William Macon wrote:


OK, thanks, I note that I was looking at an older version of the SysUtils
help. I am still getting either compiler errors or code failure as I try
different approaches with converting UnicodeString FileName, either how it's
defined, or called or used in sprintf statements. Also, it's not that the
code itself that is crashing but rather another conditional check that if it
cannot find/open the necessary files that it closes.

I am continuing to read through the current SysUtils help and other references
and try different things to get through this. I understand how to deal with
old char strings to resolve the compiler errors, but the nuances of dealing
with the old char filenames in the new Unisys functions looking for
UnicodeString is confusing. For example, I tried:
f = FileOpen(AnsiString(filename), fmOpenRead);
and that didn't work. So I'm missing something fundamental regarding how to
covert old char filenames to UnicodeString filenames, which is important for
dealing with open/read/write functions throughout the code. Any additional
suggestions or guidance would be helpful.

It is impossible to help, unless you show the exact code you are using and the
exact error message you get.

FileOpen(AnsiString(filename), fmOpenRead);

don't make much sense FileOpen takes a UnicodeString, so converting it
to AnsiString before it is converted to UnicodeString is just extra work.

P.s. String was shorthand for AnsiString in C++Builder before CB2009
and in CB2009 and later String is shorthand for UnicodeString.

Best regards
Asger
Jan Dijkstra

Posts: 206
Registered: 11/4/99
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 3, 2017 7:22 AM   in response to: William Macon in response to: William Macon
William Macon wrote:

I am continuing to read through the current SysUtils help and other references and try different things to get through this. I understand how to deal with old char strings to resolve the compiler errors, but the nuances of dealing with the old char filenames in the new Unisys functions looking for UnicodeString is confusing. For example, I tried:
f = FileOpen(AnsiString(filename), fmOpenRead);
and that didn't work. So I'm missing something fundamental regarding how to covert old char filenames to UnicodeString filenames, which is important for dealing with open/read/write functions throughout the code. Any additional suggestions or guidance would be helpful.

Since UnicodeString has overloaded constructors for, among others, char *, it's not needed to do anything special at all with respect to the FileOpen call. Just passing the char * parameter along should trigger the creation of a temporary UnicodeString, which is then initialised with the string pointed to by the char *

Which is exactly what my XE8 compiler does.

char *ptr = "old 8 bit string";
int handle = FileOpen (ptr, fmOpenRead);

is all that's needed.
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 3, 2017 1:45 PM   in response to: William Macon in response to: William Macon
William wrote:

I am now stuck on the program crashing after opening (I do get the
opening screen) because it's not opening a file that is there.

Where and what is "crashing" exactly? If your code is actually crashing
simply because load_project() returns -1, then you have a bug in your code
that is calling load_project().

f = FileOpen(filename, fmOpenRead);

So, "filename" is a const AnsiString FileName per the
SysUtils.FileOpen syntax.

In pre-2009 versions, yes. But FileOpen() has always taken a System::String
as input, and in CB2009+ that is now UnicodeString, no longer AnsiString.
You are passing in a char*, so you are invoking an Ansi->Unicode conversion
at runtime. If your filename contains any non-ASCII characters, you might
be ending up with a Unicode string that is different than you are expecting,
causing FileOpen() to fail to find the target file.

I have tried changing char *filename to AnsiString filename but that
causes an error. I tried changing it to char filename[255] but that
had no effect.

What error exactly? A compiler error, or a runtime error? You need to be
more specific. char[]/char* and AnsiString can be assigned to a UnicodeString
without causing a compiler error, and there won't be a runtime error unless
memory cannot be allocated. But you might not end up with the output you
are expecting.

Why do I keep getting a return value of -1 for "f" that indicates that
it could not obtain a file handle and an error occurred?

You tell us. What does "filename" contain to begin with? What does GetLastError()
return when FileOpen() fails? We can't debug your project for you, you have
to debug it yourself.

And BTW, FileOpen() returns a THandle, which is an unsigned int in C++, not
a signed int.

A previous find_file function in the code works for the file in question,
and the code continues to this next load_project function which does
not work.

Then you clearly have a problem when preparing the "filename" parameter value,
but you did not show that code.

--
Remy Lebeau (TeamB)
William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 8, 2017 6:46 AM   in response to: William Macon in response to: William Macon
I'm providing some more code snippets for reference:

void __fastcall TMainmenu::FormCreate(TObject *Sender)
{
char aif[255];
...
//LOOK FOR AND IF FOUND THEN LOAD AI PROJECT
if (find_file(aif))
{
ret = load_project(aif);
if (ret != 1)
{
ShowMessage("File Not Found!");
Mainmenu->Close();
}
//CLEAN LOG FILE
DeleteFile(header.logfile);
}

int find_file(char filename[255])
{
TSearchRec sr;

if (FindFirst(".
Data
scripts
*.ai", faAnyFile, sr) == 0)
{
sprintf(filename, ".
Data
scripts
%s", sr.Name);
FindClose(sr);
return 1;
}
return 0;

int load_project(char *filename)
{
int c = 0;
int f = 0;
int buff = 0;
//OPEN FILE
f = FileOpen(filename, fmOpenRead);
if (f == -1)
return -1;
//READ HEADER
buff = FileRead(f, &header, sizeof(struct header));
if (buff < 0)
{
FileClose(f);
return -2;
}
FileClose(f);
return 1;
}

In summary, the code compiles OK and find_file works because the program advances to load_project, but FileOpen fails, returning -1 which prompts the "File Not Found!" message and the program closes.

William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 8, 2017 6:48 AM   in response to: William Macon in response to: William Macon
I changed char aif[255]; to String aif;, and load_project(char *filename) to load_project(String filename) and got errors:

if (find_file(aif))
E2034 Cannot convert 'UnicodeString' to 'char *'
E2342 Type mismatch in parameter 'filename' (wanted 'char *', got 'UnicodeString')

ret = load_project(aif);
E2034 Cannot convert 'UnicodeString' to 'char *'
E2342 Type mismatch in parameter 'filename' (wanted 'char *', got 'UnicodeString')

If I don't change char aif[255]; to String aif; in the TMainmenu function, I get linking errors:
[ilink32 Error] Error: Unresolved external 'load_project(char *)' referenced from ... \DEBUG_BUILD\WGSMAIN.OBJ
[ilink32 Error] Error: Unable to perform link

My fundamental issue is understanding how to best migrate my old 'char *' filenames to 'UnicodeString' as needed and ensure smooth transfers of file information between the various code functions. I need to understand more about Ansi->Unicode conversions invoked at runtime and whether I need to make changes or not, and what changes to make and where. The old program filenames do not contain any non-ASCII characters so that shouldn't be a problem? I've been reviewing the Unicode Technical Overview, the old bcbjournal article Migrating to Unicode, and other references but it's still confusing. I managed to work through enough of a partial migration to get the code to compile, so I think if I can manage to work through a few of these runtime issues with filenames and sprintf messages then I should be able to work through the rest. All suggestions are appreciated, and I will keep trying to work through this. Thank you for your patience; this is a hobby effort and I'm not a professional C++ programmer.

Asger Joergensen

Posts: 370
Registered: 11/18/08
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 8, 2017 7:17 AM   in response to: William Macon in response to: William Macon
Hi William

Do not use the FormCreate event, it is a Delphi thing
in C++ we use the constructor!!

try something like this:

__fastcall TMainmenu::TMainmenu(TComponent* Owner)
{
   String aif;
...
   //LOOK FOR AND IF FOUND THEN LOAD AI PROJECT
   if ( FileExists( aif ) )
   {
      ret = load_project(aif);
      if (ret != 1)
      {
         ShowMessage("File Not Found!");
         Mainmenu->Close();
      }
      //CLEAN LOG FILE
      DeleteFile(header.logfile);
   }
 
int load_project(String filename)
{
   int c = 0;
    //OPEN FILE
   int f = FileOpen(filename, fmOpenRead);
 
   if (f == -1)
      return -1;
 
   //READ HEADER           // where and what is header ????????????
   int buff = FileRead(f, &header, sizeof(struct header));
 
   if (buff < 0)
   {
      FileClose(f);
      return -2;
   }
 
   FileClose(f);
   return 1;
}


Best regards
Asger
William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 9, 2017 2:31 PM   in response to: Asger Joergensen in response to: Asger Joergensen
Asger Joergensen wrote:
Do not use the FormCreate event, it is a Delphi thing
in C++ we use the constructor!!

Now I am concerned about how much else may be throughout this legacy code that may be old Borland Delphi included with C++, migrated to CodeGear C++Builder 2007 and now I'm stuck with it trying to keep it going? Unless use of FormCreate and others absolutely will not work, I'd prefer to focus on partial migrations to fix the specific char/UnicodeString problems. This shouldn't be so difficult, should it?

I made another change to int find_file(String filename) to make it consistent, but then getting errors for doing that:

sprintf(filename, ".
Data
scripts
%s", sr.Name);
E2034 Cannot convert 'UnicodeString' to 'char *'
E2342 Type mismatch in parameter '__buffer' (wanted 'char *', got 'UnicodeString')

sprint is looking for char but I now have filename as UnicodeString, so I've tried messing with swprintf and trying char and AnsiString with filename. I just get other compile errors. Clear suggestions for dealing with these char -> UnicodeString conversions and their use in functions and sprintf statements would be most helpful. Thanks.
Asger Joergensen

Posts: 370
Registered: 11/18/08
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 9, 2017 2:41 PM   in response to: William Macon in response to: William Macon
Hi William

William Macon wrote:

sprintf(filename, ".
Data
scripts
%s", sr.Name);

try this instead:

filename = L"Data
scripts
" + sr.Name;

.
is redundant, . mean the directory you are already in.

Best regards
Asger
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: FileOpen Question [Edit]
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 10, 2017 11:11 AM   in response to: William Macon in response to: William Macon
William wrote:

Now I am concerned about how much else may be throughout this
legacy code that may be old Borland Delphi included with C++, migrated
to CodeGear C++Builder 2007 and now I'm stuck with it trying to keep
it going? Unless use of FormCreate and others absolutely will not work,
I'd prefer to focus on partial migrations to fix the specific
char/UnicodeString problems. This shouldn't be so difficult,
should it?

You should NEVER use the OnCreate (and OnDestroy) event in C++, in ANY version.

sprintf(filename, ".
Data
scripts
%s", sr.Name);
E2034 Cannot convert 'UnicodeString' to 'char *'
E2342 Type mismatch in parameter '__buffer' (wanted 'char *', got 'UnicodeString')

The Clang compiler is able to validate the data types used for printf-style
function parameters at compile-time. In this case, sr.Name is a UnicodeString,
but the "%s" format specifier is expecting a char* instead. That is a mismatch,
which is why the compiler complains.

If you need to leave your 'filename' variable declared as a char[], you need
to use one of the following statements instead:

sprintf(filename, ".\\Data\\scripts\\%s", AnsiString(sr.Name).c_str());


sprintf(filename, ".\\Data\\scripts\\%S", sr.Name.c_str());


Otherwise, if you change your 'filename' variable to wchar_t[], you can then
use this statement instead:

swprintf(filename, L".\\Data\\scripts\\%s", sr.Name.c_str());


Otherwise, if you change your 'filename' variable to System::String (ie UnicodeString),
you can then use one of these statements instead:

filename.sprintf(_D(".\\Data\\scripts\\%s"), sr.Name.c_str());


filename = Format(_D(".\\Data\\scripts\\%s"), ARRAYOFCONST(( sr.Name )) );


filename = _D(".\\Data\\scripts\\") + sr.Name;


sprint is looking for char but I now have filename as UnicodeString, so
I've tried messing with swprintf and trying char and AnsiString with filename.
I just get other compile errors. Clear suggestions for dealing with these
char -> UnicodeString conversions and their use in functions and sprintf
statements would be most helpful.

It is clear from all the trouble you are having that you are trying to migrate
a lot of legacy code that simply is not ready for Unicode to begin with.
You shouldn't have needed to re-write such large sections of code, you could
have continued using your 'char'-based code as-is, just account for UnicodeString
where needed by explicitly using AnsiString wherever AnsiString was previously
being used implicitly.

--
Remy Lebeau (TeamB)


---
This email has been checked for viruses by AVG.
http://www.avg.com

William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question [Edit]
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 11, 2017 4:46 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
You should NEVER use the OnCreate (and OnDestroy) event in C++, in ANY version.

OK, I tried changing
void __fastcall TMainmenu::FormCreate(TObject *Sender)
to
__fastcall TMainmenu::TMainmenu(TComponent* Owner)
as suggested but got errors:
E2171 Body has already been defined for function '_fastcall TMainmenu::TMainmenu(TComponent *)'
E2251 Cannot find default constructor to initialize base class 'TForm'

So what else should I be doing to replace FormCreate? Or is this OK and it's just OnCreate and OnDestroy to never use?

The Clang compiler is able to validate the data types used for printf-style
function parameters at compile-time. In this case, sr.Name is a UnicodeString,
but the "%s" format specifier is expecting a char* instead. That is a mismatch,
which is why the compiler complains.

If you need to leave your 'filename' variable declared as a char[], you need
to use one of the following statements instead:
sprintf(filename, ".
Data
scripts
%s", AnsiString(sr.Name).c_str());
sprintf(filename, ".
Data
scripts
%S", sr.Name.c_str());

Thank you, AnsiString(sr.Name).c_str() worked. I had tried using AnsiString before but did not include the .c_str() piece.

You shouldn't have needed to re-write such large sections of code, you could
have continued using your 'char'-based code as-is, just account for UnicodeString
where needed by explicitly using AnsiString wherever AnsiString was previously
being used implicitly.

Yes, I would prefer keeping the legacy 'char' code as-is and using AnsiString as needed, I just wasn't clear on where and how exactly. I went back and removed some of the other changes where I redefined 'char' as 'String' and it's working. At least for this initial FileOpen problem. I am bumping into new problems trying to open/read other data files, but understanding better how to account for UnicodeString now. And I know what I need to study some more. Thanks.
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: FileOpen Question [Edit]
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 12, 2017 9:30 AM   in response to: William Macon in response to: William Macon
William wrote:

OK, I tried changing
void __fastcall TMainmenu::FormCreate(TObject *Sender)
to
__fastcall TMainmenu::TMainmenu(TComponent* Owner)
as suggested but got errors:

E2171 Body has already been defined for function '_fastcall
TMainmenu::TMainmenu(TComponent *)'

Did you include a cooresponding declaration in the .h file? It is not enough
to just rename the implementation, you have to add a declaration for it (and
remove the old declaration).

E2251 Cannot find default constructor to initialize base class 'TForm'

TForm does not have a default constructor, so you have to use an initializer
list in your constructor to construct the base class:

__fastcall TMainmenu::TMainmenu(TComponent* Owner)
    : TForm(Owner) // <-- here
{
}


--
Remy Lebeau (TeamB)


---
This email has been checked for viruses by AVG.
http://www.avg.com

William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 12, 2017 8:43 AM   in response to: William Macon in response to: William Macon
And another follow-up. I'm looking at additional FileOpen and FileRead operations. For example:

int load_data(char filename[255])
f = FileOpen(filename, fmOpenRead);
buff = FileRead(f, &check, sizeof(int));
buff = FileRead(f, &dbparms, sizeof(struct db_parms));

I'd like to continue using filename as a char AnsiString, but I need to convert this to a UnicodeString filename for FileOpen. So should I use 'UnicodeString filename', or 'UnicodeString(filename)', or something else in FileOpen? Or redefine filename prior to FileOpen? Sorry, I am still confused about how exactly this needs to be done in the syntax.

For the 'sizeof' functions, I'm wondering if I should be changing these to 'StrLen' or 'Length', and what the syntax should be to avoid errors regarding undefined functions. I saw another comment elsewhere about adding #include <System.AnsiStrings.hpp> to the code for implementing older AnsiString RTL functions, so I'm wondering what else I might be needing to make changes?
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 12, 2017 9:26 AM   in response to: William Macon in response to: William Macon
William wrote:

I'd like to continue using filename as a char AnsiString

Although you certainly can, why would you want to? Since FileOpen() expects
a UnicodeString, you are just going to force a runtime conversion from Ansi
to Unicode using the default OS codepage, so you run the risk of the Unicode
filename being different than the Ansi filename.

but I need to convert this to a UnicodeString filename for FileOpen.

Just pass the char[]/AnsiString directly to FileOpen(), the RTL will handle
the conversion for you.

So should I use 'UnicodeString filename', or 'UnicodeString(filename)', or
something else in FileOpen?

You should change the filename parameter to UnicodeString (better, to System::String):

int load_data(const System::String &filename)
{
    int f = FileOpen(filename, ...);
}


But, if not, you can just pass it as-is, it will still "work" since UnicodeString
has a constructor for char* data:

int load_data(char filename[255])
{
    int f = FileOpen(filename, ...);
}


BTW, you do realize that 255 is too low? The max path length on Windows
is 260 instead.

For the 'sizeof' functions, I'm wondering if I should be changing
these to 'StrLen' or 'Length', and what the syntax should be to
avoid errors regarding undefined functions.

sizeof only works on data types known at compile-time. sizeof(char*) is
very different than sizeof(char[255]), and strlen() on a char[255] holding
fewer than 255 chars is different than sizeof(char[255]) itself. So yes,
given a char[] array, you would typically use strlen() (lowercase, from the
C runtime library) or StrLen() (uppercase, from the Delphi RTL) instead.
Using (Ansi|Unicode)String, you would use its Length() member method instead.

I saw another comment elsewhere about adding #include <System.AnsiStrings.hpp>
to the code for implementing older AnsiString RTL functions

For most AnsiChar/AnsiString-based Delphi RTL functions (not C runtime functions),
yes. However, in the case of the Delphi RTL StrLen(AnsiChar*) function in
particular, no. It is still in System.SysUtils.hpp.

--
Remy Lebeau (TeamB)


---
This email has been checked for viruses by AVG.
http://www.avg.com

William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 23, 2017 12:27 PM   in response to: William Macon in response to: William Macon
I am still struggling with the FileOpen issues I'm having, but I think the problem may be with misreading of filenames. Code snippets below include some ShowMessage lines I added for troubleshooting:

void __fastcall TPlayers::load_scenario_descriptions()
{
int c = 0;
int r = 0;
int save = -1;
char temp[255];

if (ComboBox3->ItemIndex != -1)
save = ComboBox3->ItemIndex;

ComboBox3->Items->Clear();
for(c = 0; c < MAXSCENARIOS; c++)
{
if (strlen(game_def.scenarios[c]))
{
ShowMessage("strlen(game_def.scenarios[c]) OK");
// sprintf(temp, ".
Data
databases
%s", game_def.scenarios[c]);
sprintf(temp, ".
Data
databases
%s", AnsiString(game_def.scenarios[c]).c_str());
ShowMessage(temp);
r = area_load_area_header(temp);
if (r <= 0)
{
ShowMessage("area_load_area_header r <= 0");
memset(game_def.scenarios[c], 0, 255);
}
...
int area_load_area_header(char filename[255])
{
int f = 0;
int buff = 0;

//OPEN FILE
f = FileOpen(filename, fmOpenRead);
if (f == -1)
{
ShowMessage("area_load_area_header f == -1");
return -1;
}

I get to the load_scenario_descriptions function and the strlen check is OK. ShowMessage(temp) should then be giving me the filename to be used for the subsequent area_load_area_header function. I should be seeing "eianw001.ged" as the filename, but all I'm getting is ".\Data\databases\e" and nothing after the first character "e". I did make a change to the sprintf for AnsiString and perhaps the syntax is wrong? But it compiles and runs. Also, since there are two of these files (001 and 002), the code does cycle through both with the same result. I get the "area_load_area_header f == -1" message both times, indicating a failure for FileOpen.

I should be getting implicit AnsiString to UnicodeString conversions, yes? Based on comments above, I thought this approach should work. Either my AnsiString syntax is not correct or perhaps there is another issue? Hopefully there are enough clues here now to help me get past this filename and FileOpen problem.
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 24, 2017 11:34 AM   in response to: William Macon in response to: William Macon
William wrote:

if (strlen(game_def.scenarios[c]))

The only way that call can compile is if scenarios[c] is a char*, which means
the following AnsiString cast would be redundant:

sprintf(temp, ".\\Data\\databases\\%s", AnsiString(game_def.scenarios[c]).c_str());


In that case, your original (commented out) code would be correct:

sprintf(temp, ".\\Data\\databases\\%s", game_def.scenarios[c]);


Now, that being said, given you are formatting the data into a fixed char[]
buffer without any bounds checking, you have a potential buffer overflow,
depending on the actual length of the string that scenarios[c] is pointing
at (and your memset() calls imply that it can be as many as 254 chars in
length, which are you prefixing with 17 additional chars, so temp[] would
have to be at least 272 chars in size).

I strongly suggest you enable bounds checking, either like this:

sprintf(temp, ".\\Data\\databases\\%.*s", sizeof(temp)-17, game_def.scenarios[c]);


Or like this:

snprintf(temp, sizeof(temp), ".\\Data\\databases\\%s", game_def.scenarios[c]);


Or better, by allocating 'temp' dynamically:

AnsiString temp;
temp.sprintf(".\\Data\\databases\\%s", game_def.scenarios[c]);
r = area_load_area_header(temp.c_str());


I get to the load_scenario_descriptions function and the strlen check
is OK. ShowMessage(temp) should then be giving me the filename to
be used for the subsequent area_load_area_header function. I should
be seeing "eianw001.ged" as the filename, but all I'm getting is
".\Data\databases\e" and nothing after the first character "e".

The only way that can happen is if either:

- scenarios[c] is a wchar_t* (in which case, the call to strlen() would not
compile at all, as you would have to use wcslen() instead).

- scenarios[c] is a char* that is pointing at 16-bit character data instead
of 8-bit character data.

I'm guessing the latter is the case in this situation. You probably made
a change elsewhere in code that produces a Unicode string instead of an Ansi
string, and then you are pointing scenarios[c] to that Unicode data (using
a type-cast to avoid a compiler error, or an incorrect memory buffer copy,
or the like).

What does the code look like that is preparing scenarios[c]?

Most 16-bit strings contain some amount of null bytes in them (and that is
especially true for ASCII strings in Unicode format), so a 16-bit string
misinterpretted as an 8-bit string would produce the kind of truncated result
you are seeing. In this case, because the 2nd byte of the 1st 16bit char
would be 0 and mistaken for a null terminator.

I did make a change to the sprintf for AnsiString and perhaps the syntax
is wrong?

No, the syntax is fine, and the resulting data would not be truncated like
you are seeing, if it were proper 8-bit data to begin with. Your symptom
is suggestive of 16-bit data being used instead.

Based on comments above, I thought this approach should work.

It should be, yes. Which implies that there is something wrong elsewhere
in code that you have not accounted for yet.

--
Remy Lebeau (TeamB)
William Macon

Posts: 19
Registered: 2/24/16
Re: FileOpen Question
Click to report abuse...   Click to reply to this thread Reply
  Posted: Apr 24, 2017 2:20 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:

In that case, your original (commented out) code would be correct:

I strongly suggest you enable bounds checking
Or better, by allocating 'temp' dynamically:

AnsiString temp;
temp.sprintf(".\\Data\\databases\\%s", game_def.scenarios[c]);
r = area_load_area_header(temp.c_str());


I'm guessing the latter is the case in this situation. You probably made
a change elsewhere in code that produces a Unicode string instead of an Ansi
string, and then you are pointing scenarios[c] to that Unicode data (using
a type-cast to avoid a compiler error, or an incorrect memory buffer copy,
or the like).
Remy, thank you, this worked. I had made some other fixes to other functions dealing with filenames to resolve compiler errors. I updated the sprintf statement for this function and then went back to update other similar sprintf statements in a couple other functions.

There's a comment in the "Migrating legacy C++Builder Apps to C++ Builder 10 Seattle" article about searching for all occurrences of sprintf for manual checks. I am now at the point where I need to do this to keep moving forward and I now better understand what syntax to use. This has been very challenging, since none of the migration references provide suggestions like you have provided here? Certainly a lot of insights have been gained since Josh Kelly wrote his bcbjournal article back in 2010. More migration information would be helpful. Regardless, the help here on the forum has been very good and I thank you again for your patience.
Legend
Helpful Answer (5 pts)
Correct Answer (10 pts)

Server Response from: ETNAJIVE02