Commit Graph
30 Commits
Author SHA1 Message Date
Kyle Ambroff-Kao fb0b77979a loopback: Bump default MTU from 16KiB to 64KiB
While profiling with iperf for #18203 I noticed that the default MTU for
the loopback interface is 16KiB. Other operating systems set this quite
a bit higher. Linux sets it to 64KiB by default. Random Google searches
seem to suggest that Microsoft sets it even higher than this although I
don't have a Windows computer to verify this at the moment.

Changing the default to 64KiB in this patch makes a pretty big
difference in a kvm VM with a single CPU. The max throughput goes from
about 2Mbps/s to around 12Mbps/s, around a 6x increase.

With the same VM but 8 CPUs, the throughput goes from 1.58Gbps to
1.99Gbps, about a 26% increase in throughput. It seems the throughput
is a little more stable too and doesn't drop periodically. I suspect
that this is just because there is less CPU saturation in the loop
consumer thread.

With window size maxed out to around 1GiB as described in #15886 there
is still about a 10% increase in throughput with this change.

There are still some weird performance issues to diagnose but this seems
like a better default.

Change-Id: I8c5d088298a4a7b3e8b1aa1a2f4f85b0cc9c62c2
Reviewed-on: https://review.haiku-os.org/c/haiku/+/6000
Reviewed-by: waddlesplash <[email protected]>
2023-01-15 05:34:08 +00:00
Kyle Ambroff-Kao 107712bf01 AutoRaise: Properly load tray icon bitmap
AutoRaise needs to load bitmaps stored as resources in the AppRaise
image. The strategy it used before was to store the path to itself in
its settings file.

It stored the entry_ref to the image in the settings, which may not be
stable across reboots with packagefs. In addition, the path could
change at some point for any other reason.

This patch changes AutoRaise to just use the same method as other tray
applets, which is to search the process for the image containing the
AutoRaise code, and then to get the path from the image_info.

Most of the initialization work is performed by TrayView::_init(),
which is called by the constructor. Some of the operations performed
by this method can fail, leaving TrayView partially initialized. Any
error is handled by returning early after telling Deskbar to remove
this item so that it will not continue running in a partially
initialized state. Deskbar will invoke the destructor so even a
partially initialized TrayView will be cleaned up.

Fixes #16473

Change-Id: Id1ac1ef78d410a7d191cfe88f1ba1db30e258adc
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3341
Reviewed-by: Jérôme Duval <[email protected]>
2020-10-22 17:52:53 +00:00
Kyle Ambroff-Kao e0e5d12ea3 test/haikudepot: Fix build of haikudepottest.so
hrev54429 added some changes to logging in HaikuDepot which broke the
build of the tests.

The build for haikudepottest.so adds some cpp files from the
HaikuDepot source. Now that Logger::NameForLevel and
Logger::IsLevelEnabled are referenced in some of the code being tested
we need to include Logger.cpp as well.

This just adds Logger.cpp to the haikudepottest.so build.

Change-Id: I80a8bb2053c3c78d821ce3cf9902c3a42b0def37
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3056
Reviewed-by: Adrien Destugues <[email protected]>
2020-07-23 07:23:37 +00:00
Kyle Ambroff-Kao 38963e7596 SecureSocket: Handle interrupted reads and writes
If a system call performed by SSL_read is interrupted by a signal, it
seems to set its error to SSL_ERROR_WANT_READ. This triggers logic
added in hrev53853 which assumes the caller is doing async reads and
returns B_WOULD_BLOCK.

This breaks uses of BSecureSocket that do blocking reads.

* Detect interrupted signal by checking for EINTR in errno.
* Adding this retry loop to BScureSocket::Write as well since it can
  have the same problem.

Resolves issue #15853.

Change-Id: I8198a8496fa3a2ccee00bda87375a482a0d4ba3d
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2825
Reviewed-by: Adrien Destugues <[email protected]>
2020-05-29 14:35:14 +00:00
Kyle Ambroff-Kao 57672294e9 tests/storage: Fix BDirectory tests
After this patch "UnitTester BDirectory" passes.

Most of this test suite already documented the differences in behavior
between BeOS R5 and Haiku. I verified whether these comments were
accurate and removed the cases which handle BeOS specific behavior.

Most of the differences are just Haiku using more specific errors:

* Initializing BDirectory with an entry that is not a directory
  results in B_NOT_A_DIRECTORY.
* There is obviously no /boot/beos. Use /boot/system for this test
  instead. BDirectory::IsRootDirectory returns true for this path
  since it is the root of the system package.
* Initializing to child path "" results in B_ENTRY_NOT_FOUND instead
  of successful initialization with B_OK only to later return
  B_BAD_VALUE if the BDirectory is used.
* BDirectory::Find(NULL, BEntry*) doesn't touch the BEntry parameter
  since the provided path is NULL, where BeOS R5 will set the BEntry's
  status to B_BAD_VALUE.
* Clean up -Wparentheses warnings for assertions of the form
  CPPUNIT_ASSERT(path == existingSub == B_OK), which is another way of
  saying path != existingSub. This is because the path ends up being a
  normalied path, but the input path is not. For example
  /tmp/existing-dir becomes /boot/system/cache/tmp/existing-dir. I
  verified that this is the same behavior as BeOS, and then added some
  normalized paths to compare against.

Change-Id: I5125ef221fba92793959efead96d7daaa181a119
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2826
Reviewed-by: Adrien Destugues <[email protected]>
2020-05-29 14:34:47 +00:00
Kyle Ambroff-Kao 51dd385e3e tcp: Don't skip TIME_WAIT state for loopback sockets
This fixes a SEGFAULT in the tcp add-on reported in issue #15952. See
that issue for some analysis.

The short version is that, when closing a session over the loopback
interface, there is a special branch which skips the TIME_WAIT state
and instead just releases the socket while handling a RST/ACK
segment. If the timing is right this can lead to the reference
counts becoming imbalanced, leading to the code in tcp_receive_data
segfaulting when it tries to release the reference it acquired from
EndpointManager::FindConnection.

I can't find any other systems which skip the TIME_WAIT state with
loopback sessions, and I'm not entirely certain that it's a totally
safe thing to do anyway. This patch instead just treats local sessions
the same way it does a remote session and uses the TIME_WAIT state.

Any workload which creates and discards lots of ephemeral sockets can
just use SO_REUSEADDR to handle this situation like any other system.

To add a final bit of safety, the only place where a net_socket can be
used after calling gSocketModule->release_socket(net_socket*) is in
tcp_receive_data(). release_socket() returns true if the reference
count falls to zero, deleting the socket. There was an unused segment
action flag DELETE_ENDPOINT that I renamed to DELETED_ENDPOINT, which
is used by tcp_receive_data to know whether its safe to release its
reference to the socket after calling TCPEndpoint::SegmentReceived().

Change-Id: I2652fb225c3c8419234cfd627f74ff2de8402003
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2793
Reviewed-by: Axel Dörfler <[email protected]>
Reviewed-by: waddlesplash <[email protected]>
2020-05-24 15:47:57 +00:00
Kyle Ambroff-Kao 44a4bc5fd6 tcp: Remove sanity checks from BufferQueue in release builds
Each TCPEndpoint has two BufferQueue members, one for the send queue
and one for the receive queue.

If DEBUG_BUFFER_QUEUE is enabled, then most methods of BufferQueue
call BufferQueue::Verify(), sometimes twice. This member function
performs some sanity checking which requires iterating through every
net_buffer in the queue.

Disabling this in a debug build improved throughput by a factor of 5x
over the loopback interface on my laptop. Using iperf the measured
throughput went from 900Mbps to around 4.8Gbps.

This patch turns this sanity checking off for release builds.

* Rename DEBUG_BUFFER_QUEUE to DEBUG_TCP_BUFFER_QUEUE
* Change the default in BufferQueue.h to disabled
* Set DEBUG_TCP_BUFFER_QUEUE to KDEBUG_LEVEL_2 in
  kernel_debug_config.h

Change-Id: I262dac5d7e2889d2942bbdcf6b667cc0cbafa4c8
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2780
Reviewed-by: waddlesplash <[email protected]>
2020-05-23 02:32:58 +00:00
Kyle Ambroff-Kao e1b7c1c7ac storage/SymLink: Fix Be API regression in ReadLink
After this patch, "UnitTester BSymLink" passes.

BSymLink::ReadLink() in BeOS would always return the length of the
link unless an error occurred. Before this patch, Haiku instead seemed
to emulate posix readlink() behavior, returning the number of bytes
copied into the output buffer.

BeOS also did not guarantee that the string written into the output
buffer is NULL terminated if the output buffer cannot contain the
entire link contents, but the Haiku implementation does since it is is
a basic safety issue.

This patch fixes this and updates the Haiku API docs to describe the
behavior explicitly.

Fixing this required changing behavior in bfs_read_link, which
required changes in many more places.

docs/user/storage/SymLink.dox:
src/kits/storage/SymLink.cpp:
* Don't return B_BUFFER_OVERFLOW if the provided buffer is not large
  enough to hold the link contents.
* Update documentation to clearly describe behavior.

src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp:
* Change bfs_read_link() to always return the link length. This is
  called by common_read_link in the VFS, which is called by
  _kern_read_link().

src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp:
src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp:
src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp:
src/add-ons/kernel/file_systems/iso9660/kernel_interface.cpp:
src/add-ons/kernel/file_systems/netfs/client/netfs.cpp:
src/add-ons/kernel/file_systems/nfs/nfs_add_on.c:
src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp:
src/add-ons/kernel/file_systems/reiserfs/Iterators.cpp:
src/add-ons/kernel/file_systems/reiserfs/Iterators.h:
src/add-ons/kernel/file_systems/reiserfs/Volume.cpp:
src/add-ons/kernel/file_systems/reiserfs/Volume.h:
* Update the implementation of read_link for these filesystems. Some
  of them were incorrect, and some had just copied the posix behavior of
  bfs from before this patch.
* Use user_memcpy in ext2_read_link()
* Use user_memcpy in nfs fs_read_link()
* Use user_memcpy in reiserfs StreamReader::_ReadIndirectItem and
  StreamReader::_ReadDirectItem
* Remove unused method Volume::ReadObject in reiserfs.

src/add-ons/kernel/file_systems/packagefs/nodes/UnpackingLeafNode.cpp:
src/add-ons/kernel/file_systems/packagefs/package_links/PackageLinkSymlink.cpp:
* Update UnpackingLeafNode::ReadSymlink and
  PackageSymLink::ReadSymLink() to set the bufferSize out parameter to
  the symlink length. Both of these are called by
  packagefs_read_symlink.
* Use user_memcpy

src/add-ons/kernel/file_systems/netfs/client/netfs.cpp:
* netfs seems mostly unimplemented. Added a FIXME note for future
  implementers so that they know to implement the correct behavior.

src/system/libroot/posix/unistd/link.c:
* readlinkat() was just wrapping _kern_read_link() because before this
  patch it had expected posix behavior. But now it does not, so we
  need to return the number of bytes written to the output
  buffer.

src/build/libroot/fs.cpp:
* Update _kern_read_link() in the compatibility code to emulate the
  Haiku behavior on the host system. This is done by using an
  intermediate buffer that is guaranteed to fit the link contents and
  returning its length. The intermediate buffer is copied into the
  output buffer until there is no more room.

src/tests/kits/storage/SymLinkTest.cpp:
* This patch also resolves some test failures similar to those
  resolved in ee8cf35f0 which fixed tests for BNode. The tests were
  failing because Haiku's error checking is just better.

  BeOS allowed constructing a BSymLink with BSymLink(BDirectory*,
  const char*) with the entry name of "". The same is true of the
  equivilant SetTo() method. The BSymLink object will appear valid
  until you attempt to use it by, for example, calling the ReadLink
  method, which will return B_BAD_VALUE.

  Haiku does a more appropriate thing and returns B_ENTRY_NOT_FOUND,
  for this constructor and the equivilant SetTo(BDirectory*, const
  char*) method. This patch fixes these test assertions to match Haiku
  behavior.

docs/develop/file_systems/overview.txt:
* Add notes for future filesystem driver implementers to call this
  mistake when implementing fs_vnode_ops::read_symlink.

docs/user/drivers/fs_interface.dox:
* Fix documentation for fs_vnode_ops::read_symlink

Change-Id: I8bcb8b2a0c9333059c84ace15844c32d4efeed9d
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2502
Reviewed-by: waddlesplash <[email protected]>
Reviewed-by: Axel Dörfler <[email protected]>
2020-04-30 10:13:41 +00:00
Kyle Ambroff-Kao 6f9f7e02a8 tests/HttpTest: Fix build on x86_gcc2
* std::istreambuf_iterator<T> template isn't available until C++11.
* std::vector<T>::cbegin() is not available
* Add missing include of errno.h

Change-Id: Ice344f6b0f93bf72d9120674607878c4c3e8ef54
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2515
Reviewed-by: waddlesplash <[email protected]>
2020-04-26 16:53:07 +00:00
Kyle Ambroff-Kao ee8cf35f07 bfs: Return B_NAME_TOO_LONG from BPlusTree::Find
This patch makes "UnitTester BNode" pass.

Several tests which attempt to create or access a directory entry that
exceeds the maximum length assume that B_NAME_TOO_LONG status will be
returned, since that is what BeOS does. When constructing a BNode with
a path like "/tmp/some really long filename larger than 256
characters...", the vfs eventually calls bfs_lookup() which calls
BPlusTree::Find(). In the case of a really long entry, Find() returns
B_BAD_VALUE.

This patch just changes BPlusTree::Find to return the more specific
error that matches BeOS.

Additionally this patch fixes some assertions in NodeTest. BeOS seems
to have been missing some error checking code in the initialization of
BNode, specifically with BNode(Directory*, const char*) and the
equivalent SetTo method. If you provide an empty string for the child
entry name to either of those, B_OK will be returned. But either way
you initialize the object, when you try to use it, like with
BNode::GetAttrInfo(), you'll get B_BAD_VALUE.

This just changes any assertions for this situation to expect
B_ENTRY_NOT_FOUND, which is the actual initialization error Haiku
sets.

This and the change to bfs resolves many assertions the following
storage tests:
* TestCaller BFile::Init Test 1
* TestCaller BFile::Init Test 2
* TestCaller BNode::Init Test1
* TestCaller BNode::Init Test2
* TestCaller BSymLink::Init Test 1
* TestCaller BSymLink::Init Test 2

Change-Id: I8598352aa341ffcab9f7bc3e6740ae1cb3dbab8c
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2490
Reviewed-by: waddlesplash <[email protected]>
2020-04-18 18:22:04 +00:00
Kyle Ambroff-Kao 40aa43588d tests/net: Fix NetworkAddressTest::TestUnset()
This test just checks a default-constructed BNetworkAddress and a
BNetworkAddress that has had its Unset method invoked are in the same
state.

It also compares against a BNetworkAddress that has been constructed
with the AF_INET family. In BeOS R5 this worked because you could
construct a BNetworkAddress with nullptr for the host parameter, but
in Haiku this this leads to Unset() being invoked. So
BNetworkAddress(AF_INET, NULL) is the same as a default-constructed
BNetworkAddress object.

This patch just changes the test to construct the BNetworkAddress used
for comparison with a valid host using the IPv4 loopback address
instead.

Change-Id: Id890110cfa1f3c40a630f9005e2a390e25f6baae
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2388
Reviewed-by: waddlesplash <[email protected]>
2020-04-04 22:44:37 +00:00
Kyle Ambroff-Kao 2a0d1eb890 makeudfimage: Remove this tool completely
The AllocatorTest suite fails, and when investigating why it was
broken I realized that it was testing code that has been dead and
unused for quite some time. makeudfimage hasn't been touched since
2004 (5f2185ea0), and was removed from the build in 2006 (5adca30a1).

The only changes that have been made to it since then are build fixes
for the tests. Adding it back to the build would require some work
since many things have changed since then.

Since there are many other tools and libraries out there which can
make UDF images (and which have been ported to Haiku), this code
doesn't really seem worth maintaining at this point.

This patch just removes it, as well as the associated tests.

Change-Id: I23da8df83b7f141b3394a022030545d42a287881
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2332
Reviewed-by: Adrien Destugues <[email protected]>
2020-03-09 12:18:06 +00:00
Kyle Ambroff-Kao 2ac34dee51 tests/net: HTTP proxy client test
With this patch, ProxyTest is implemented and all of the tests in
HttpTest are enabled.

Adding a transparent proxy server implementation proxy.py. Like
testserver.py, this can be provided a socket file descriptor and port
via command-line arguments.

TestServer was refactored to extract ChildProcess and
RandomTCPServerPort, which are now also used by TestProxyServer.

ProxyTest starts TestProxyServer and validates that the request is
sent to the proxy and is routed to the appropriate endpoint of the
downstream server.

The template which adds common tests between HttpTest and HttpsTest
was changed slightly to just take a BThreadedTestCaller<T>&, which
made it simpler to add additional test cases to one suite which are
not appropriate to the other. There wasn't much point in keeping that
template as a member function so I moved it into HttpTest.cpp as a
free function template.

Change-Id: Ied32d6e10bb195d111cae7bbcf0e93168118088b
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2291
Reviewed-by: Adrien Destugues <[email protected]>
2020-02-29 00:27:36 +00:00
Kyle Ambroff-Kao 762f26bac8 tests/net: Implement testserver.py TLS for HttpsTests
This patch is a followup to 0dde5052b which added testserver.py, a
HTTP echo server for the HttpTests and HttpsTests in the ServicesKit
test suite. This patch implements `testserver.py --use-tls` which
allows for re-enabling HttpsTests.

If `--use-tls` is used, then a self-signed TLS certificate is
generated in a temporary directory which is used by the test
server. This option is used when running HttpsTests.

There doesn't seem to be a good way to have these tests trust the
certificate generated by this test at the moment. Until that API
exists I've just made these tests ignore certificate validation. We'll
want to resolve this and update these tests to actually verify that
validation works as expected.

Some minor tweaks had to be made to testserver.py to take care of
differences in the response body when serving HTTP and HTTPS requests.

Some additional changes:
* Don't depend on any files outside of src/tests/kits/net/service for
  these tests. UploadTest was uploading a file from /boot, but I
  copied it into the test source directory to avoid having these tests
  break if someone makes an unrelated change. It doesn't really matter
  what the contents of this file is as long as it doesn't change.
* Use BThreadedTestCase. This speeds up the tests considerably, mostly
  because it means that the different test cases can share the same
  HttpTest instance, which means there is only a single TestServer
  instance, and it takes around half a second to bootstrap the test
  server on my system, and even longer if --use-tls is used.

Change-Id: I6d93d390ebd56115365a85109140d175085e1f01
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2260
Reviewed-by: Adrien Destugues <[email protected]>
2020-02-23 07:58:58 +00:00
Kyle Ambroff-Kao 0dde5052bb tests/net: Working integration tests for HTTP client
This patch is part 1 of 3 with the goal of having a working
integration test harness for BHttpRequest. In this patch the existing
test cases were expanded and fixed for HTTP. In followup patches the
test harness will be updated to support HTTPS and reverse proxies.

Before this patch the tests for BHttpRequest had hard dependencies on
the external services httpbin.org and portquiz.net. These tests
eventually stopped working because the owner of those services made
changes, causing the assertions in these tests to fail.

The goal of these patches is to make a test harness that allows for
the same kinds of end-to-end integration tests but without any
external dependencies.

The test suite now includes a Python script called testserver.py which
is a HTTP echo server of sorts. When it receives a request, it will
echo the request headers and request body back to the client as a
text/plain response body.

The TestServer class manages the lifecycle of this testserver.py
process. Each test case calls Start() on the server to start a new
instance, and then it is shut down when the destructor is called. On
each invocation a random port is assigned by the kernel in TestServer,
and that socket file descriptor is provided to the child testserver.py
script.

Authorization tests are supported, currently implementing Basic and
Digest auth. If the test server receives a request for a path
/auth/<auth-scheme>/<expected-username>/<expected-password>, then the
appropriate authorization scheme will be employed. For example, if
/auth/basic/foo/bar is used as the path, then the server will expect
the Authorization header to contain an appropriate Basic auth
payload.

The tests now perform a bit more validation than before, validating
the expected HTTP headers and response body is returned from the
server.

The following tests are not fixed yet or were removed:
* PortTest was removed entirely since I'm not sure of the point of this
  test, and that functionality seems to be covered by the existing tests
  anyway.
* HTTPS tests are not functional yet, but will be in a followup
  patch. THis requires updating testserver.py to generate a
  self-signed TLS cert if --use-tls is provided.
* ProxyTest was disabled before this patch, but can be enabled in a
  followup patch by providing a reverse proxy in the test harness.

Change-Id: Ia201ef4583b7636c61e77072a03db936cb0092be
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2243
Reviewed-by: Adrien Destugues <[email protected]>
2020-02-16 08:34:08 +00:00
Kyle Ambroff-Kao 5f4f455cfd test/storage: Fix BFile tests
After this patch "UnitTester BFile" runs with no failures.

This includes some changes to NodeTest as well since the BFile suite
inherits some tests from it, but "UnitTester BNode" doesn't pass yet.

FileTest:
* Fix format strings in log messages to use the correct types and
  quite warnings.

* Test for expected behavior with BFile(..., B_READ_ONLY | B_ERASE_FILE)

  There is a table in the BFile tests which is used as input for a
  series of tests which construct BFile objects with various
  permutations of the mode parameter.

  For two of these, the combination of B_ERASE_FILE and B_READ_ONLY
  are used for the file mode. The system rejects these with the error
  B_NOT_ALLOWED. This is enforced in bfs_open(), which looks for that
  particular combination (O_RDONLY | O_TRUNC).

  I tested this on BeOS R5 and it will indeed let you create a
  BFile(path, B_READ_ONLY | B_ERASE_FILE) which basically replaces the
  file at path with an empty file.

  The Haiku behavior seems way more sensible, so I'm changing these
  tests to match its current behavior.

* BFile(..., B_READ_ONLY).SetSize() isn't allowed.

  BeOS R5 allowed you create a BFile with the B_READ_ONLY flag and
  then successfully set the size of the file with
  BFile::SetSize(). Haiku doesn't, and that seems way better. Updating
  BFileTest::SizeTest to match Haiku behavior.

  One whole sub-test was removed from FileTest::SizeTest because it
  calls SetSize() on a `B_READ_ONLY | B_ERASE_FILE` BFile. This is
  redundant as other tests in this suite already verify that it is not
  possible to construct a BFile with that combination of flags.

NodeTest:
* Fix tests to match Haiku's max attribute length.

  Many of these tests assume that the max size of an attribute name is
  255 bytes, and it was in BeOS R5, which I just confirmed.

  But Haiku allows 256 bytes. In 4069e1f30 some compromise was struck
  that allowed this to avoid breaking userspace which had been allowed
  to use 256 byte keys at some point.

* WriteAttr, ReadAttr and RemoveAttr all return B_NAME_TOO_LONG if an
  attribute name longer than 256 bytes (excluding null terminator) are
  provided.

* Disable NodeTest::AttrRenameTest since attr rename is not supported

  Tests in BNodeTest (inherited by BFile tests) exercize
  BNode::RenameAttr(), but from what I can see attribute renaming is
  not implemented at all. bfs_rename_attr() has a TODO comment and
  just always returns EOPNOTSUPP (B_NOT_SUPPORTED). And that is the
  value returned from BNode::RenameAttr() in these tests.

  So for now I made these tests just check that B_NOT_SUPPORTED is
  returned from BNode::RenameAttr(), so when this functionality is
  implemented these tests will fail and can be cleaned up.

Change-Id: I6cfe90ca45f3a8afa709edc9b85e648fdc865e82
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2182
Reviewed-by: Adrien Destugues <[email protected]>
2020-02-05 14:54:18 +00:00
Kyle Ambroff-Kao e7fd54fcde tests/haikudepot: Fix build of ValidationUtilsTest
Changeset 835e7239d added some additional tests for HaikuDepot, but
ValidationUtilsTest::AddTest references a test method which doesn't
exist.

I think the intention was to have TestEmailInvalid defined, but it was
split into more granular tests like TestEmailInvalidNoDomain,
etc. Just removing this undefined test from the suite.

Change-Id: I858233df29657d07d78ad1784691572e65a26045
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2183
Reviewed-by: Adrien Destugues <[email protected]>
2020-02-03 11:36:07 +00:00
Kyle Ambroff-Kao 2112748284 tcp: Fix KDL when sockets are reused
This fixes a KDL that is triggered by the following scenario.

1. A socket is created, attempting to establish a session
   between (loopback, ephemeral port) => (remote address, remote
   port).
2. That socket ends up in the closed state because the remote is not
   accepting connections.
3. The socket is re-used to connect to a different (remote address,
   remote port).

The problem is that fConnectionHash is a BOpenHashTable<TCPEndpoint>,
and inserting endpoint multiple times can create a linked list
cycle (TCPEndpoint is an intrusive linked list node). That means that,
even though TCPEndpoint's destructor removes itself from
fConnectionHash, there will still be a pointer to it left behind,
which means that future accesses within that hash table bucket will
result in a segfault.

The added fConnectionHash.Remove(endpoint) here prevents the KDL, as
it ensures that socket reuse doesn't result in a cycle.

Fixes #13927, see that ticket for a detailed explanation of the
problem.

Also added some regression tests:
* Added SocketTests::ClientSocketReuseTest to PosixNetTest, which
  reproduces this KDL.
* BOpenHashTable: Added tests to cover RemoveUnchecked and removal
  of an object that isn't in the table.

Change-Id: If4bcc1e0d94350a5ad9ba8e7ae6f1b783b3f6d34
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2173
Reviewed-by: Stephan Aßmus <[email protected]>
2020-02-01 11:52:34 +00:00
Kyle Ambroff-Kao e8a484b0f6 tests/kernel: Fix KPath tests
42e3c6f97 changed the default buffer size for KPath, but the tests
still assume the original default.

Change-Id: I2712d81bbdc678bbaae11e975b0a9a05f5a9151f
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2175
Reviewed-by: Axel Dörfler <[email protected]>
2020-01-31 15:22:55 +00:00
Kyle Ambroff-Kao aa7ac127f8 tests/kernel: Add tests for BOpenHashTable
Linking libkernelutilstest.so with libbe to for use of BObjectList in
the tests.

Change-Id: I1abb991e240dd522821a71ef54d22a1ca7957283
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2165
Reviewed-by: Adrien Destugues <[email protected]>
2020-01-30 00:30:00 +00:00
Kyle Ambroff-Kao 8d763b7f0d Terminal: Fix crash for keystroke with unexpected modifiers
This patch fixes a segfault in Terminal that happens when you issue a
keystroke with unexpected modifiers. An easy way to repro this is:

 1. Set command key to super
 2. Set option key to meta in Terminal
 3. Issue `ctrl+alt+shift+<key>`

In DefaultState::KeyDown(), the branch that handles option key as meta
detects that additional modifiers were used along with the meta key
and tries to look up the keymap associated with that combination of
modifier keys.

This lookup in TermView::fKeymapTableForModifierse can return NULL,
since TermView::SetKeymap() sets a pre-defined set of (modifier combo)
=> keymap entries.

Change-Id: I3ce4a7cff6c84913d99507e44849f9b048769f67
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2138
Reviewed-by: waddlesplash <[email protected]>
2020-01-22 00:28:25 +00:00
Kyle Ambroff-Kao e8b9c0e1a6 tests/BHandlerTester: Fix Perform1 test case
TBHandlerTester::Perform1() tests the result of calling
BHandler::Perform() with invalid input. On BeOS R5 this returns
B_ERROR, but on Haiku this delegates to BArchivable::Perform(), which
returns B_NAME_NOT_FOUND.
Change-Id: Ia05220da4155a629cf611fec76da6477b9f1dbf4
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2137
Reviewed-by: Adrien Destugues <[email protected]>
2020-01-22 00:27:31 +00:00
Kyle Ambroff-Kao 654135466f support/BPath: Flatten should work on empty BPath
BPath::Flatten() on an empty path returns B_OK in BeOS R5, just
writing an empty entry_ref to the povided buffer. The Haiku
implementation has some additional validation that causes B_NO_INIT to
be returned instead.

This patch attempts to recreate the same behavior of BeOS in this
situation.

* Don't check for initialization in BPath::Flatten(). Instead,
  just write an empty entry_ref to the provided buffer if the BPath is
  empty.
* Fix estimation of expected size when testing the return value of
  BPath::FlattenedSize().
* Clean up warning by removing unecessary forward-declaration of
  CppUnit::Test.

Change-Id: I88880cbb298bdcb594c9c8fef48314165c49e9e5
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2115
Reviewed-by: Adrien Destugues <[email protected]>
Reviewed-by: Axel Dörfler <[email protected]>
2020-01-21 09:36:42 +00:00
Kyle Ambroff-Kao 7037b5d9bc support/Archivable: Reset errno in validate_instantiation()
validate_instantiation(BMessage*, const char*) sets errno for invalid
input, or if the requested class is not found, but it doesn't reset
errno to B_OK if validation succeeds.

I verified that in BeOS R5, errno is set to B_OK if
validate_instantiation succeeds.

This fixes BHandler::Instantiate2 and BHandler::Instantiate3 tests.

Change-Id: I531777e6ba47e9635da2da1fc8c8103bb233b0f3
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2136
Reviewed-by: Axel Dörfler <[email protected]>
Reviewed-by: Adrien Destugues <[email protected]>
2020-01-21 08:46:20 +00:00
Kyle Ambroff-Kao dcfd4f324e GetRecentTester: Fix path to test apps
get_test_app_ref() computes a path relative to
BTestShell::GlobalTestDir() that is supposed to contain test apps that
will be launched with launch_test_app(). The path is incorrect so
several tests fail.

The test apps are actually just in BTestShell::GlobalTestDir(). Fixing
this resolves 6 of the 12 failing tests in the BRoster tests.

Change-Id: I4b287c19fd83d3afe40dca137fea2bd61a0f9359
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2114
Reviewed-by: Adrien Destugues <[email protected]>
Reviewed-by: Axel Dörfler <[email protected]>
2020-01-16 10:31:15 +00:00
Kyle Ambroff-Kao 4df8f0223b tests: Fix build on x86_gcc2
This patch fixes the build of unittests on x86_gcc2.

src/tests/kits/shared/KeymapTest.cpp:
* Don't use auto
* Don't use braced-initialization for std::map.

src/tests/system/kernel/vm/Jamfile:
* Link lock.o from kernel source to include _mutex_lock and
  _mutex_unlock when linking libkernelvmtest.so

Change-Id: I60e02bfb23334064ec25d767f659a188e393ed1c
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2074
Reviewed-by: Adrien Destugues <[email protected]>
2020-01-04 08:29:41 +00:00
Kyle Ambroff-Kao 2c17ecf993 tests/kits/support/bmemoryio: Fix WriteTest
This patch fixes a bug in the BMemoryIO unit tests that made them
fail and adds an additional test case for read-only buffers.

The failing test case invokes BMemoryIO::WriteAt() with the position
parameter set to -10, which is invalid and should result in a return
value of B_BAD_VALUE. And it does, but the test fails because it was
testing for the return value 5, as in 5 bytes copied.

An additional test case is added for read-only BMemoryIO objects. If
the BMemoryIO(const void*, size_t) constructor is used then it will be
marked as read-only, so writes should fail with B_NOT_ALLOWED.

Change-Id: Icf4b837c77fba2be958f9d3e4b3adb18a23b037f
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2066
Reviewed-by: Adrien Destugues <[email protected]>
Reviewed-by: waddlesplash <[email protected]>
2020-01-03 03:30:35 +00:00
Kyle Ambroff-Kao fbc30e9145 support/String: Revert 6c67c7d63 to fix double-free
6c67c7d63 was attempting to fix a leak caught by a static analysis
tool, but it actually just introduced a double-free bug. Running
`UnitTester BString` will result in a crash.

The original code was correct because, in the event that realloc()
fails in BString::_Resize(), the value of fPrivateData is still
retained. It will be freed by the destructor of BString only if
fPrivateData is not shared by another BString instance, since BStrings
are copy-on-write.

Note that while the change in 6c67c7d63 caused tests to fail, that
doesn't mean those tests are ideal. They only trigger
BString::_Resize() to fail because they depend on implementation
details of hoard2 which limits allocations via malloc() to
1GB. Most malloc() implementations will allow allocations of arbitrary
sizes using anonymous mappings (mmap on Linux, or create_area() in
Haiku). This is a much bigger change, so for now I'm just adding some
comments so that we can revisit these tests if we make a change to the
allocator.

Change-Id: I208c1c7a76b6b4409d237b911c62bb3198e49dab
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2060
Reviewed-by: Stephan Aßmus <[email protected]>
2019-12-31 11:03:59 +00:00
Kyle Ambroff-Kao 39147a617f write_overlay: Fix SMAP violation in OverlayInode::Read
Change-Id: I66d852008a4375bb9295bc0238ed6289fb8439d5
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2046
Reviewed-by: Jérôme Duval <[email protected]>
2019-12-22 11:00:48 +00:00
Kyle Ambroff-Kao d8779e44ee Drop requirement for specifying firmware map size
The HAIKU_FIRMWARE_NAME_MAP macro takes a size parameter to define the
firmware map array type, and then a multi-dimensional array literal is
assigned to the array defined by that macro.

This is error-prone. The idualwifi7260 driver, before this patch, had
the size incorrectly set to 6 when the number of entries was 7, which
sliced the last entry off of the map, making it unavailable to the
driver. After fixing this size, the driver properly loads the
iwm-8265-22.ucode firmware on my computer.

This patch changes that macro to take a const char[][2] literal as its
only parameter, making it less likely for this sort of bug to be
re-introduced.

Fixes #15413.

Change-Id: I78a75e692a8637af0f13d1eb16180ce8d95d0852
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1917
Reviewed-by: Adrien Destugues <[email protected]>
2019-10-16 00:23:55 +00:00