Opened 14 months ago

Closed 14 months ago

Last modified 14 months ago

#15353 closed build error (fixed)

usage of wxFFile not protected by #ifdef wxUSE_FFILE

Reported by: jroemmler Owned by: vadz
Priority: low Milestone:
Component: build Version: 2.9.5
Keywords: Cc:
Blocked By: Blocking:
Patch: yes

Description

If setup.h contains this line:

#define wxUSE_FFILE 0

(instead of the 1 as per default), then src\common\debugrpt.cpp fails to compile with an unprotected usage of wxFile, wxFFileInputStream, wxFFileOutputStream etc. from ffile.h:

For example on Windows x64:
[...]

cl /c /nologo /TP /Fovc_mswud_x64\qalib_debugrpt.obj /MDd /DWIN32  /Zi \
   /Fd..\..\lib\vc_x64_lib\wxmsw29ud_qa.pdb  /D_DEBUG /Od \
   /D_CRT_SECURE_NO_DEPRECATE=1 /D_CRT_NON_CONFORMING_SWPRINTFS=1 \
   /D_SCL_SECURE_NO_WARNINGS=1 /D__WXMSW__ /D_UNICODE  \
   /I..\..\lib\vc_x64_lib\mswud /I..\..\include  /W4  /DWXBUILDING \
   /I..\..\src\tiff\libtiff /I..\..\src\jpeg /I..\..\src\png \
   /I..\..\src\zlib /I..\..\src\regex /I..\..\src\expat\lib \
   /GR /EHsc /Yu"wx/wxprec.h" /Fp"vc_mswud_x64\wxprec_qalib.pch" \
   ..\..\src\common\debugrpt.cpp
debugrpt.cpp
..\..\src\common\debugrpt.cpp(295) : error C2065: 'wxFFile' : undeclared identifier
..\..\src\common\debugrpt.cpp(295) : error C2146: syntax error : missing ';' before identifier 'file'
..\..\src\common\debugrpt.cpp(295) : error C3861: 'file': identifier not found
..\..\src\common\debugrpt.cpp(296) : error C2228: left of '.IsOpened' must have class/struct/union type
        type is ''unknown-type''
..\..\src\common\debugrpt.cpp(296) : error C2228: left of '.Write' must have class/struct/union type
        type is ''unknown-type''
..\..\src\common\debugrpt.cpp(633) : error C2065: 'wxFFileOutputStream' : undeclared identifier
..\..\src\common\debugrpt.cpp(633) : error C2146: syntax error : missing ';' before identifier 'os'
..\..\src\common\debugrpt.cpp(633) : error C3861: 'os': identifier not found
..\..\src\common\debugrpt.cpp(649) : error C2065: 'wxFFileInputStream' : undeclared identifier
..\..\src\common\debugrpt.cpp(649) : error C2146: syntax error : missing ';' before identifier 'is'
..\..\src\common\debugrpt.cpp(649) : error C3861: 'is': identifier not found
..\..\src\common\debugrpt.cpp(650) : error C2228: left of '.IsOk' must have class/struct/union type
        type is ''unknown-type''
..\..\src\common\debugrpt.cpp(650) : error C2228: left of '.IsOk' must have class/struct/union type
NMAKE : fatal error U1077: 'cl' : return code '0x2'
Stop.

Attachments (1)

wxUSE_FFILE0.patch download (11.3 KB) - added by jroemmler 14 months ago.
Protect places using wxFFile etc. with #if wxUSE_FFILE …

Download all attachments as: .zip

Change History (10)

comment:1 Changed 14 months ago by jroemmler

There is a related problem when compiling src/richtext/richtextbuffer.cpp:8328:
if wxUSE_FFILE is set to 0, then this member function is not available and must not be called:

wxRichTextFileHandler::LoadFile(wxRichTextBuffer* buffer, const wxString& filename)

The same goes for wxRichTextFileHandler::SaveFile in line 8343.

comment:2 follow-up: Changed 14 months ago by jroemmler

The story continues with compilation of

..\..\src\stc\stc.cpp(4404) : error C2065: 'wxFile' : undeclared identifier

comment:3 in reply to: ↑ 2 Changed 14 months ago by jroemmler

Replying to jroemmler:

The story continues with compilation of

..\..\src\stc\stc.cpp(4404) : error C2065: 'wxFile' : undeclared identifier

Ohh, no. This is slightly different: a copy'n'paste programming bug in
src\stc\stc.cpp line 53: must be

    #include "wx/file.h"

instead of

    #include "wx/ffile.h"

comment:4 Changed 14 months ago by vadz

  • Priority changed from normal to low

It's practically impossible to maintain all the 2^N combinations of the build systems so we have to rely on people interested in building wxWidgets with some atypical configuration to make the necessary changes to make this work. So if you could please submit a patch adding the missing wxUSE_FILE checks, it would be welcome. TIA!

comment:5 follow-up: Changed 14 months ago by jroemmler

  • Patch set

Ok, I was hoping that pointing you to the troublesome combination out of 2N would be sufficient :-)
I'm not actively involved into wx-dev, so I did my best to fix the problem by looking at other places in the code which had to deal with the same problem.
The resulting patch is attached (a cygwin patch against the windows new-line 2.9.5 stable release branch). There might be some hassles with line endings (a unix2dos or dos2unix may be required to apply it successfully).

What the patch does: it adds

#if wxUSE_FFILE
// wxFFile usage here...
#elif wxUSE_FILE
// wxFile usage here..
#endif

where possible.

If it was not possible, there was a choice of either (1) disabling the code for the whole implementation, like

#if wxUSE_FFILE
int class::method() {
  //wxFFile usage here ...
#endif
}

..so code calling that function will fail to compile without proper #if ... (=> the problem escalates up the call tree)...

or (2) providing a dummy implementation that returns with an error condition, like

bool class::method() {
#if wxUSE_FFILE
  //wxFFile usage here ...
#else
  return false;
#endif
}

(=> problem propagation stops here).

I've seen both in the code, so I wasn't really sure. Please check the patch carefully.
I also noticed a subtle difference in wxFile::Write() and wxFFile::Write().
They differ in the default of their 2nd argument:

bool 	Write (const wxString &s, const wxMBConv &conv=wxConvUTF8)
bool 	Write (const wxString &s, const wxMBConv &conv=wxConvAuto())

Is that by intention?

Changed 14 months ago by jroemmler

Protect places using wxFFile etc. with #if wxUSE_FFILE ...

comment:6 in reply to: ↑ 5 Changed 14 months ago by vadz

  • Owner set to vadz
  • Status changed from new to accepted

Replying to jroemmler:

The resulting patch is attached (a cygwin patch against the windows new-line 2.9.5 stable release branch).

Thanks! It looks good to me so I'll apply it soon with just some minor changes.

There might be some hassles with line endings (a unix2dos or dos2unix may be required to apply it successfully).

Yes, I'll do that. FWIW, patches/files with Unix EOLs are preferred but this is not a problem either way.

What the patch does: it adds

#if wxUSE_FFILE
// wxFFile usage here...
#elif wxUSE_FILE
// wxFile usage here..
#endif

where possible.

If it was not possible, there was a choice of either (1) disabling the code for the whole implementation ... or (2) providing a dummy implementation that returns with an error condition

I think (1) is generally preferable.

I also noticed a subtle difference in wxFile::Write() and wxFFile::Write().
They differ in the default of their 2nd argument:

bool 	Write (const wxString &s, const wxMBConv &conv=wxConvUTF8)
bool 	Write (const wxString &s, const wxMBConv &conv=wxConvAuto())

Is that by intention?

No, but there was no real difference as wxConvAuto always uses UTF-8 on output, its main usefulness is when reading data. But we did change this already and now wxConvAuto is consistently used in both places.

Thanks again for your patch!

comment:7 Changed 14 months ago by VZ

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [74596]) Fix build with wxUSE_FFILE=0.

Add the missing "#if wxUSE_FFILE" checks and add fallbacks to wxFile if it's
available.

Closes #15353.

comment:8 Changed 14 months ago by jroemmler

Vadim,

thanks a lot for the extremely fast review!

comment:9 Changed 14 months ago by VZ

(In [74632]) Reflect changes in stc.cpp in stc.cpp.in from which it's generated.

This should have been part of r74596, see #15353.

Note: See TracTickets for help on using tickets.