Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#12280 closed Bug report (rejected)

libfilezilla 0.24.1 Solaris fixes

Reported by: Thomas Klausner Owned by:
Priority: normal Component: libfilezilla
Keywords: Cc:
Component version: Operating system type: Other
Operating system version:

Description

I'm not using Solaris myself, but a Solaris user of pkgsrc reported two problems with compiling libfilezilla: struct sigaction needs "struct" in front, and wcsnrtombs needs to be prefixed with "std::".

The attached patches fix the issues.

Thanks.

Attachments (2)

patch-lib_socket.cpp (557 bytes ) - added by Thomas Klausner 2 months ago.
patch-lib_string.cpp (534 bytes ) - added by Thomas Klausner 2 months ago.

Download all attachments as: .zip

Change History (10)

by Thomas Klausner, 2 months ago

Attachment: patch-lib_socket.cpp added

comment:1 by Thomas Klausner, 2 months ago

Please ignore the patch for lib/string.cpp; I did more testing and it breaks the build on NetBSD.
The other patch still looks valid to me.

by Thomas Klausner, 2 months ago

Attachment: patch-lib_string.cpp added

comment:2 by Thomas Klausner, 2 months ago

I have replaced the lib/string.cpp patch with one that limits the change to Solaris.

comment:3 by Tim Kosse, 2 months ago

Status: newmoreinfo

The patch to socket.cpp has been applied.

The patch to string.cpp seems wrong. Instead, could you please try if #including wchar.h works?

comment:4 by Thomas Klausner, 2 months ago

Status: moreinfonew

Thanks for applying the patch.

I can't test myself, but I asked someone else who can and they reported:

Doesn't help, it's already being included via this path:

string.cpp: In function 'std::string fz::to_string(const wstring_view&)':
string.cpp:362:17: error: 'wcsnrtombs' was not declared in this scope; did you mean 'std::wcsnrtombs'?

362 | size_t len = wcsnrtombs(nullptr, &in_p, inlen, 0, &ps);

| ~
| std::wcsnrtombs

In file included from /usr/include/wchar.h:31,

from /opt/local/gcc9/include/c++/9.3.0/cwchar:44,
from /opt/local/gcc9/include/c++/9.3.0/bits/postypes.h:40,
from /opt/local/gcc9/include/c++/9.3.0/bits/char_traits.h:40,
from /opt/local/gcc9/include/c++/9.3.0/string:40,
from /opt/local/gcc9/include/c++/9.3.0/stdexcept:39,
from /opt/local/gcc9/include/c++/9.3.0/array:39,
from /opt/local/gcc9/include/c++/9.3.0/tuple:39,
from /opt/local/gcc9/include/c++/9.3.0/functional:54,
from /opt/local/gcc9/include/c++/9.3.0/pstl/glue_algorithm_defs.h:13,
from /opt/local/gcc9/include/c++/9.3.0/algorithm:71,
from libfilezilla/string.hpp:6,
from string.cpp:1:

/usr/include/iso/wchar_iso.h:310:15: note: 'std::wcsnrtombs' declared here

310 | extern size_t wcsnrtombs(char *_RESTRICT_KYWD, const wchar_t _RESTRICT_KYWD,

| ~

comment:5 by Tim Kosse, 2 months ago

Status: newmoreinfo

Something else must be amiss then. According to https://docs.oracle.com/cd/E88353_01/html/E37843/wcsnrtombs-3c.html wcsnrtombs is declared if wchar.h is included.

Is it hidden behind some obscure #ifdef?

comment:6 by Thomas Klausner, 2 months ago

Status: moreinfonew

Their reply:

It is declared, it isn't hidden (see the "declared here"), it's just that
it's under namespace std, which is why when I change it to std:: it works.

comment:7 by Tim Kosse, 2 months ago

Resolution: rejected
Status: newclosed

wchar.h is a C-header, C does not have namespaces. wcsnrtombs is defined in POSIX.1-2008 to be declared through wchar.h. Solaris claims to support POSIX.1-2008.

The C++ standard does not specify std::wcsnrtombs. The C++ standard further notes under [namespace.std] the following about any non-standard extensions to the std namespace:

Unless otherwise specified, the behavior of a C++ program is undefined if it adds declarations or definitions to namespace std or to a namespace within namespace std.
[...a long list of exceptions not applicable in the context of this ticket...]

Please file a bug report against Solaris to have wcsnrtombs added in accordance to POSIX.1-2008 and another bug report to have the offensive std::wcsnrtombs declaration removed.

comment:8 by Thomas Klausner, 2 months ago

Can I ask you to reconsider?

Assuming your argument is correct, this does not help existing users/versions of Solaris. The workaround in the patch is limited to Solaris, so it does not affect other operating systems, and it's a small change.

Note: See TracTickets for help on using tickets.