Otherwise, moving the cursor (at least in active, filled-cell mode) on
to a true-coloured character cell causes it to vanish completely
because the cell's colours override the thing that differentiates the
cursor.
I'm not sure if any X11 monochrome visuals or Windows paletted display
modes are still around, but just in case they are, we shouldn't
attempt true colour on either kind of display.
I know some users don't like any colour _at all_, and we have a
separate option to turn off xterm-style 256-colour sequences, so it
seems remiss not to have an option to disable true colour as well.
This is a heavily rewritten version of a patch originally by Lorenz
Diener; it was tidied up somewhat by Christian Brabandt, and then
tidied up more by me. The basic idea is to add to the termchar
structure a pair of small structs encoding 24-bit RGB values, each
with a flag indicating whether it's turned on; if it is, it overrides
any other specification of fg or bg colour for that character cell.
I've added a test line to colours.txt containing a few example colours
from /usr/share/X11/rgb.txt. In fact it makes quite a good demo to run
the whole of rgb.txt through this treatment, with a command such as
perl -pe 's!^\s*(\d+)\s+(\d+)\s+(\d+).*$!\e[38;2;$1;$2;$3m$&\e[m!' rgb.txt
This causes PuTTY processes spawned from its system-tray menu to run
with the -restrict-acl option (or rather, the synonymous &R prefix
used by my auto-constructed command lines for easier parsing).
The previous behaviour of Pageant was never to pass -restrict-acl to
PuTTY, even when started with -restrict-acl itself; this is not
actually a silly thing to want to do, because Pageant might well have
more need of -restrict-acl than PuTTY (it stores longer-term and more
powerful secrets) and conversely PuTTY might have more need to _not_
restrict its ACL than Pageant (in that among the things enabled by an
unrestricted ACL are various kinds of accessibility software, which is
more useful on the more user-facing PuTTY than on Pageant).
But for those who want to lock everything down with every security
option possible (even though -restrict-acl is only an ad-hoc
precaution and cannot deliver any hard guarantees), this new option
should fill in the UI gap.
After a conversation this week with a user who tried to use it, it's
clear that Borland C can't build the up-to-date PuTTY without having
to make too many compromises of functionality (unsupported API
details, no 'long long' type), even above the issues that could be
worked round with extra porting ifdefs.
64-bit PuTTY should be loading gssapi64.dll, not gssapi32.dll. (In
contrast to the Windows system API DLLs, such as secur32.dll which is
also mentioned in the same source file; those keep the "32" in their
name whether we're in Win32 or Win64.)
Like every other toolchain I've tried, my Coverity scanning build has
its share of random objections to parts of my Windows API type-
checking system. I do wonder if that bright idea was worth the hassle
- but it would probably cost all the annoyance all over again to back
out now...
Ilya Shipitsin sent me a list of errors reported by a tool 'cppcheck',
which I hadn't seen before, together with some fixes for things
already taken off that list. This change picks out all the things from
the remaining list that I could quickly identify as actual errors,
which it turns out are all format-string goofs along the lines of
using a %d with an unsigned int, or a %u with a signed int, or (in the
cases in charset/utf8.c) an actual _size_ mismatch which could in
principle have caused trouble on a big-endian target.
Rather than loading some from spoolss.dll, which some MSDN documentation
suggests, but which doesn't work very well in practice.
(Also, remove an unused variable.)
I've also been experimenting recently with running Wix on Linux under
Mono, rather than running it on Windows in the obvious way. Wix under
Mono insists on forward slashes in pathnames, and it turns out that
Wix on Windows doesn't object to them either, so I can safely change
them over unconditionally and then my installer source will work in
both modes.
stdint.h is one of the set of standard C headers that has more to do
with the compiler than the library, and hence, clang provides its own
version of it, even when you're using it in clang-cl mode with Visual
Studio headers, and even when those headers are old enough not to have
a stdint.h of their own. So in clang builds, including stdint.h is
always the right way to get uintptr_t defined.
Patch due to Brian K. White; we now condition our own declaration of
DLL_DIRECTORY_COOKIE on whether the toolchain's headers had #defined
it already, rather than trying to guess the answer to that from
version-number macros.
(Apparently everything that defines DLL_DIRECTORY_COOKIE does it by
does seem to work in all my tests.)
When making select_result() return void (a2fb1d9), I removed a "return"
at the end of the FD_CLOSE case, causing a fallthrough into FD_ACCEPT
with hilarious (segfaulting) consequences. Re-instate the "return".
plug_receive() and plug_closing() return 0 or 1 depending on whether
they think the main connection has closed. It is not appropriate, as
handle_gotdata and handle_socket_unfreeze did, to treat them as
returning a backlog. In fact, plugs are unusual in PuTTY in not
reporting a backlog, but just calling into the socket to freeze and
unfreeze it as required.
It's redundant with back->connected(): only the SSH backend has a
receive function that can ever return 0, and whenever ssh_receive
returns 0 it has called ssh_do_close, which will cause future calls
to ssh_connected also to return 0. Similarly, all backend closing
functions ensure that future calls to their connected function will
return 0.
This too is not in the list of known DLLs on Windows 10. I don't know
of any actual viable hijacking attack based on it, which according to
my reading of MSDN (specifically, a rather vague hint in
https://msdn.microsoft.com/library/ff919712) _may_ be because we
mention the common controls assembly in our application manifest; but
better safe than sorry.
Now the entire list of remaining DLLs that PuTTY links against at load
time is a subset of the Win10 known DLLs list, so that _should_ mean
that everything we load before we've deployed our own defence
(SetDefaultDllDirectories) is defended against for us by Windows
itself.
The printing functions are split between winspool.drv and spoolss.dll
in a really weird way (who would have guessed that OpenPrinter and
ClosePrinter don't live in the same dynamic library?!), but _neither_
of those counts as a system 'known DLL', so linking against either one
of these at load time is again a potential DLL hijacking vector.
It's not on the default list of important system 'known DLLs' stored
at HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\KnownDLLs (see
https://isc.sans.edu/forums/diary/DLL+hijacking+vulnerabilities/9445/ )
which apparently makes it exempt from Windows's standard DLL hijacking
defence, i.e. if an executable links against it in the normal way then
that executable will be vulnerable to DLL hijacking from a file called
winmm.dll in the same directory as it.
The solution is to load it dynamically _after_ we've locked down our
DLL search path, which fortunately PuTTY's code base is well used to
doing already for other DLLs.
This gives me an extra safety-check against having mistyped one of the
function prototypes that we load at run time from DLLs: we verify that
the typedef we defined based on the prototype in our source code
matches the type of the real function as declared in the Windows
headers.
This was an idea I had while adding a pile of further functions using
this mechanism. It didn't catch any errors (either in the new
functions or in the existing collection), but that's no reason not to
keep it anyway now that I've thought of it!
In VS2015, this automated type-check works for most functions, but a
couple manage to break it. SetCurrentProcessExplicitAppUserModelID in
winjump.c can't be type-checked, because including <shobjidl.h> where
that function is declared would also bring in a load of other stuff
that conflicts with the painful manual COM declarations in winjump.c.
(That stuff could probably be removed now we're on an up-to-date
Visual Studio, on the other hand, but that's a separate chore.) And
gai_strerror, used in winnet.c, does _have_ an implementation in a
DLL, but the header files like to provide an inline version with a
different calling convention, which defeats this error-checking trick.
And in the older VS2003 that we still precautionarily build with,
several more type-checks have to be #ifdeffed out because the
functions they check against just aren't there at all.
If MIT Kerberos is installed, then using GetProcAddress to extract
GetUserNameExA() from secur32.dll causes Windows to implicitly load
sspicli.dll in turn - and it does it in a search-path-unclean way.
If we load it in our own way before that happens, then Windows doesn't
need to load it again and won't do so wrongly.
[SGT: tidied up commit message from original patch]
gssapi32.dll from MIT Kerberos as well as from Heimdal both load
further DLLs from their installation directories.
[SGT: I polished the original patch a bit, in particular replacing
manual memory allocation with dup_mb_to_wc. This required a Recipe
change to link miscucs.c and winucs.c into more of the tools.]
I scrolled past it just now and decided those open braces at the ends
of the lines are just too ugly to live. They originally got that way
when I put the whole source base through GNU indent, which as far as
I'm concerned is a horrible misfeature of indent!
This is apparently the other half of what we should have done when we
called SetCurrentProcessExplicitAppUserModelID at run time: it
associates to PuTTY's Start Menu shortcut the same identifier that we
give to the running PuTTY process, so that jump lists saved under the
latter will be visible to users mousing over the former.
I've also done the same thing to the desktop shortcut, just in case
that does anything useful.
In the sentdata callback function given to handle_output_new, the
'new_backlog' parameter can be negative, and if so, it represents a
Windows error code and not a backlog size at all. handle_sentdata was
not checking for this before passing it on to plug_sent.
While I'm looking at these two dialog boxes, I notice there's another
prominent difference between PuTTY's one and these: I also never got
round to adding the button to go to PuTTY's main website. Now added.
The current About boxes are too small to fit in all the buildinfo
data, in particular the source-control commit id. Apparently I forgot
to enlarge them when I enlarged the one in PuTTY proper.
(All the same information is nonetheless *present* in the box, but
there seems to be no way to scroll a static text control, so you can
only find that out by 'Select All' and copying to the clipboard.)
Anyway. Now resized to the same dimensions as the main PuTTY About
box. (Really I should centralise more definitions into a common
resource file, but there we go.)
Conflicts in the FAQ are fixed by incorporating Jacob's rewritten
post-0.68 version. (But owing to considerable git confusion I haven't
managed to get his name on to this commit anywhere.)
When we create a socket with socket() (in try_connect, sk_newlistener, and
ipv4_is_local_addr) also call SetHandleInformation to disable handle
inheritance for this socket. This fixes dup-sessions-dont-close.
This commit also updates the dumps of Plink's and PSCP's help output,
adding the -proxycmd option to both and the -shareexists option to
Plink.
(Or rather, _re_-adding the latter, since it was introduced in error
by commit 07af4ed10 due to a branch management error and hastily
removed again in 29e8c24f9. This time it really does match reality.)
When a handle socket is in THAWING state and handle_socket_unfreeze is
gradually passing the backlogged data on to the plug, the plug might
suddenly turn round and close the socket in the course of handling
plug_receive(), which means that handle_socket_unfreeze had better be
careful not to have had everything vanish out from under it when that
call returns. To solve this, I've added a 'deferred close' flag which
handle_socket_unfreeze can set around its call to plug_receive, and
handle_socket_close will detect that and not actually free the socket,
instead leaving that for handle_socket_unfreeze to do under its own
control.
When called with -V to ask for our version, return 0 rather than 1.
This is the usual behaviour observed by ssh(1) and other Unix commands.
Also use exit() rather than cleanup_exit() in pscp.c and psftp.c ; at
this point we have nothing to cleanup!
Firstly, I had asserted that data would never arrive on a handle
socket in state FREEZING, which is just an error, because FREEZING is
precisely the state of not being quite frozen _yet_ because one last
read is still expected to arrive from the winhandl.c reading subthread
which it's too late to cancel. I meant to assert that it wasn't
FROZEN.
Secondly, when the handle socket was in state FREEZING, I failed to
actually _set_ it to FROZEN.
And thirdly, when the handle socket starts thawing again (i.e. there's
now outgoing buffer space so we can start sending our backlogged
data), I forgot to ever call bufchain_consume, so that the same block
of data would get sent repeatedly.
I can only assume that nothing I've ever done has actually exercised
this code!
I think all of the cases in this switch must have originally said
(shift_state ? 'this' : 'that'), and in all but the VK_NUMPAD5 case
the two options were different, and I left VK_NUMPAD5 containing a
redundant ?: just to make it line up in a nice table with the others.
But now the others all have more options than that because I had to
support Ctrl as well as Shift modifiers, so there's no reason to have
that silly ?: lingering around (and it annoys Coverity).
Avoided referring to some functions and header files that aren't there
in the winelib world (_vsnprintf, _stricmp, SecureZeroMemory,
multimon.h), and worked around a really amazingly annoying issue in
which Winelib objects to you using the type 'fd_set' unless you
included winsock2.h before stdlib.h.
The loops that were supposed to count up the number of buttons in the
variadic argument list forgot to increment the counter.
On the other hand, these functions aren't actually _used_ anywhere in
the current code - looks as if commit 616c837cf was the last time they
were seen - but manual dialog stuff like PuTTYgen might yet find a use
for them in future.
Coverity observes that sometimes 'struct tm' can have other fields
(e.g. glibc's tm_gmtoff), so it's as well to make sure we initialise
the whole thing to zero.
Assignments that are overwritten shortly afterwards and never used,
and a completely unused variable. Also, the bogus array access in
testbn.c could have actually accessed one beyond the array limit
(though of course it's only in a test harness).
Partly to reassure the user that they got what they asked for, and
partly so that's a clue for us in the logs when we get bug reports.
This involved repurposing platform_psftp_post_option_setup() (no longer
used since e22120fe) as platform_psftp_pre_conn_setup(), and moving it
to after logging is set up.
These are benign, I think. clang warns about casting non-pointer-sized
integers to pointers, but the Windows API actually does sometimes
involve values that are either pointers or _small_ integers, so in the
two cases involved I just cast through ULONG_PTR to silence the
warning. And clang insists that the integer whose address I give to
sk_getxdmdata is still uninitialised afterwards, which is just a lie.
clang-cl generates warnings saying they're deprecated, in favour of
the same names but prefixed with an underscore. The warnings are
coming from the standard MS headers, and I'm already #defining those
names differently on Unix, so I'll honour them.
When I added some extra braces in commit 095072fa4 to suppress this
warning, I think in fact I did the wrong thing, because the
declaration syntax I was originally using is the Microsoft-recommended
one in spite of clang not liking it - I think MS would be within their
rights (should they feel like it) to add those missing braces in a
later version of the WinSock headers, which would make the current
warning-clean code stop compiling. So it's better to put the code back
as it was, and avoid the clang warning by using clang's
warning-suppression pragmas for just those declarations.
I've also done the same thing in winnet.c, for two initialisers of
IPv6 well-known addresses which had the same problem (but which I
didn't notice yesterday because a misjudged set of Windows version
macros had prevented me from compiling that file successfully at all).
We used to offer to clean up saved sessions, so we should mention that
we don't for the benefit of users of old versions, who might have been
relying on it.
This change applies to every situation when GUI PuTTY knowingly spawns
another GUI PuTTY, to wit, the System menu options 'New Session',
'Duplicate Session' and the 'Saved Sessions' submenu.
(Literally speaking, what we actually pass through to the sub-PuTTY's
command line is not the "-restrict-acl" option itself, but a special
prefix "&R", which has the same meaning but which lives in the special
pre-argv-splitting command-line namespace like the magic options used
for Duplicate Session and the old '@sessionname' prefix which the
Saved Sessions submenu still uses. Otherwise, by the time we split up
argv and recognised -restrict-acl, it would be too late to parse those
other options.)
One case in which PuTTY spawns a subprocess and this change _doesn't_
apply is when the subprocess is a proxy command which happens to be a
Plink. Recognising Plink commands in that situation would be fragile
and unreliable, and in any case if the user wants a proxy Plink to be
ACL-restricted, they are in control of its exact command line so they
can add -restrict-acl themselves.
These ones are stylistic rather than potential bugs: mostly signedness
of char pointers in cases where they clearly aren't going to cause the
wrong thing to actually happen, and one thing in winsecur.c where
clang would have preferred an extra pair of braces around some
initialisers but it's legal with or without. But since some of clang's
warnings turn out to be quite useful, it seems worth silencing these
harmless ones so as to be able to see the rest.
I was having a play with clang's MSVC compatibility mode, just to see
how much of PuTTY it could compile, and one of its warnings pointed
out this error which must have crept in when I was changing the EOF
flags in winhandl.c from booleans to three-state enums - I left the !
on the front of what was previously an if (!thing) and needed to turn
into if (thing == EOF_NO).
Several functions were passing a 'char *error' and assigning error
messages directly into 'error', where they should have been passing
'char **error' and assigning error messages into '*error' if the error
message is to be returned to the caller. This would have led to
incomplete error messages.
As documented in bug 'win-process-acl-finesse', we've had enough
assorted complaints about it breaking various non-malicious pieces of
Windows process interaction (ranging from git->plink integration to
screen readers for the vision-impaired) that I think it's more
sensible to set the process back to its default level of protection.
This precaution was never a fully effective protection anyway, due to
the race condition at process startup; the only properly effective
defence would have been to prevent malware running under the same user
ID as PuTTY in the first place, so in that sense, nothing has changed.
But people who want the arguable defence-in-depth advantage of the ACL
restriction can now turn it on with the '-restrict-acl' command-line
option, and it's up to them whether they can live with the assorted
inconveniences that come with it.
In the course of this change, I've centralised a bit more of the
restriction code into winsecur.c, to avoid repeating the error
handling in multiple places.
The previous agent-forwarding system worked by passing each complete
query received from the input to agent_query() as soon as it was
ready. So if the remote client were to pipeline multiple requests,
then Unix PuTTY (in which agent_query() works asynchronously) would
parallelise them into many _simultaneous_ connections to the real
agent - and would not track which query went out first, so that if the
real agent happened to send its replies (to what _it_ thought were
independent clients) in the wrong order, then PuTTY would serialise
the replies on to the forwarding channel in whatever order it got
them, which wouldn't be the order the remote client was expecting.
To solve this, I've done a considerable rewrite, which keeps the
request stream in a bufchain, and only removes data from the bufchain
when it has a complete request. Then, if agent_query decides to be
asynchronous, the forwarding system waits for _that_ agent response
before even trying to extract the next request's worth of data from
the bufchain.
As an added bonus (in principle), this gives agent-forwarding channels
some actual flow control for the first time ever! If a client spams us
with an endless stream of rapid requests, and never reads its
responses, then the output side of the channel will run out of window,
which causes us to stop processing requests until we have space to
send responses again, which in turn causes us to stop granting extra
window on the input side, which serves the client right.
Now, instead of returning a boolean indicating whether the query has
completed or is still pending, agent_query() returns NULL to indicate
that the query _has_ completed, and if it hasn't, it returns a pointer
to a context structure representing the pending query, so that the
latter can be used to cancel the query if (for example) you later
decide you need to free the thing its callback was using as a context.
This should fix a potential race-condition segfault if you overload an
agent forwarding channel and then close it abruptly. (Which nobody
will be doing for sensible purposes, of course! But I ran across this
while stress-testing other aspects of agent forwarding.)
It's been commented out for ages because it never really worked, and
it's about to become further out of date when I make other changes to
the agent client code, so it's time to get rid of it before it gets in
the way.
If and when I do get round to supporting asynchronous agent requests
on Windows, it's now pretty clear to me that trying to coerce this
ghastly window-message IPC into the right shape is the wrong way, and
a better approach will be to make Pageant support a named-pipe based
alternative transport for its agent connections, and speaking the
ordinary stream-oriented agent protocol over that. Then Pageant will
be able to start adding interactive features (like confirmation
dialogs or on-demand decryption) with freedom to reply to multiple
simultaneous agent connections in whatever order it finds convenient.
backend_socket_log was generating the IP address in its error messages
by means of calling sk_getaddr(). But sk_getaddr only gets a SockAddr,
which may contain a whole list of candidate addresses; it doesn't also
get the information stored in the 'step' field of the Socket that was
actually trying to make the connection, which says _which_ of those
addresses we were in the middle of trying to connect to.
So now we construct a temporary SockAddr that points at the
appropriate one of the addresses, and use that for calls to plug_log
during connection setup.
In case of connection errors before and during the handshake,
net_select_result is retrying with the next address of the server. It
however was immediately going to the last address as it was not
checking the return value of try_connect for all intermediate
addresses.
This shows the build platform (32- vs 64-bit in particular, and also
whether Unix GTK builds were compiled with or without the X11 pieces),
what compiler was used to build the binary, and any interesting build
options that might have been set on the make command line (especially,
but not limited to, the security-damaging ones like NO_SECURITY or
UNPROTECT). This will probably be useful all over the place, but in
particular it should allow the different Windows binaries to be told
apart!
Commits 21101c739 and 2eb952ca3 laid the groundwork for this, by
allowing the various About boxes to contain free text and also
ensuring they could be copied and pasted easily as part of a bug
report.
It's a bit conceptually incoherent anyway - if you're uninstalling
PuTTY _systemwide_ across a multi-user system, it doesn't really make
sense that you'd also want to wipe the saved sessions for the
individual user running the uninstaller.
Also, making this change to the Inno Setup uninstaller opens up a
nicer migration path to MSI for people doing large corporate rollouts:
they can upgrade to this version of the Inno Setup package, then do a
silent uninstall of it (which should now _actually_ be silent, since
this cleanup step was the thing that interrupted it otherwise) and
then a silent install of the MSI.
The MSI format has a fixed field for target architecture, so there's
no way to build a single MSI that can decide at install time whether
to install 32-bit or 64-bit (or both). The best you can do along those
lines, apparently, is to have two MSI files plus a bootstrap .EXE that
decides which of them to run, and as far as I'm concerned that would
just reintroduce all the same risks and annoyances that made us want
to migrate away from .EXE installers anyway.
Uses the BUILDDIR mechanism I added to Makefile.vc in commit
d3db17f3e.
This change is purely internal to Buildscr, and shouldn't affect the
output of a build. It paves the way to have Buildscr run multiple
Windows builds using different compilers, by putting each one in a
different subdirectory so that their outputs don't collide.
A user points out that buf[] in sk_tcp_peer_info is only used in the
IPv6 branch of an ifdef, and is declared with a size of
INET6_ADDRSTRLEN, which won't be defined in NO_IPV6 mode. So moving
the definition inside another IPv6-only ifdef fixes the resulting
build failure.
The algorithm Windows uses to generate AppUserModelIDs "hangs on" to
removable media (CDs/DVDs) if PuTTY is launched with a CD/DVD in a drive.
Set the AppUserModelID explicitly to avoid using this algorithm.
At least on systems providing SetDefaultDllDirectories, this should
stop PuTTY from being willing to load DLLs from its containing
directory - which makes no difference when it's been properly
installed (in which case the application dir contains no DLLs anyway),
but does if it's being run from somewhere uncontrolled like a browser
downloads directory.
Preliminary testing suggests that this shouldn't break any existing
deliberate use of DLLs, including GSSAPI providers.
The Windows implementation of get_file_posn is calling SetFilePointer
to obtain the current position in the file. However it did not
initialize the variable holding the high order 32-bit to 0. Thus,
SetFilePointer either returned -1 to indicate an error or did move the
file pointer to a different location instead of just returning the
current position. This change just initializes the variable to 0.
As a result, this bug has caused psftp's reget command to fail
resuming transfers or to create corrupt files due to setting up an
incorrect resume offset.
Previously only Unix front ends bothered to include it, on the basis
that only the pty backend needed it (to set IUTF8 in the pty). We're
about to need it everywhere else too.
It's really only useful with MinGW rather than a Cygwin toolchain these
days, as recent versions of the latter insist against linking with the
Cygwin DLL.
(I think it may no longer be possible to build with Cygwin out of the
box at all these days, but I'm not going to say so without having
actually checked that's the case. Settle for listing MinGW first in
various comments and docs.)
I've just upgraded my build environment to the latest Inno Setup
(apparently fixing some DLL hijacking issues), and found that the
build script doesn't run any more because the name of the output file
has changed - it used to produce Output/setup.exe, but now it produces
Output/mysetup.exe.
Rather than just fixing the build script to expect the new name, I've
explicitly specified an output filename of my own choice in putty.iss,
so that the build script should now work with versions before and
after the change.
I can't believe this codebase is around 20 years old and has had
multiple giant const-fixing patches, and yet there are _still_ things
that should have been const for years and aren't.
Ahem. Cut-and-paste goof that I introduced in commit 2eb952ca3, when I
moved the application names out of separate text controls in the
resource-file dialog descriptions.
Blocking PROCESS_QUERY_INFORMATION access to the process turned out to
stop screen readers like Microsoft Narrator from reading parts of the
PuTTY window like the System Menu.
strcspn() returns a size_t, which is not safe to pass as the parameter
in a printf argument list corresponding to a "*" field width specifier
in the format string, because the latter should be int, which may not
be the same size as size_t.
We were calling Windows file-handling API functions GetFilesize and
SetFilePointer, each of which returns two halves of a large integer by
writing the high half through a pointer, with pointers to the wrong
integer types. Now we're always passing the exact type defined in the
API, and converting after the fact to our own uint64 type, so this
should avoid any risk of wrong-sized pointers.
These integer types are correct for the id/handle parameter to
AppendMenu / InsertMenu / DeleteMenu, and also for the return type of
dialog box procedures.
We also have the special-purpose -DUNPROTECT to disable just the ACL
changes, but if you want to compile without any Windows security API
support at all (e.g. experimentally building against winelib) then
it's easier not to have to specify both defines separately.