Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions library/src/calc_tables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

#include "calc_tables.hpp"
#include <bit>
#include <vector>

#include <pbn.hpp>
Expand Down Expand Up @@ -254,6 +255,22 @@ int STDCALL CalcAllTablesN(
if (count * dealsp->no_of_tables > MAXNOOFTABLES * DDS_STRAINS)
return RETURN_TOO_MANY_TABLES;

// Each hand must hold exactly 13 cards. A malformed deal otherwise reaches

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that this is what we want? I think all hands have the same number of cards and at most 13 cards is the correct criteria as partial deals are allowed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

I also prefer to hide this feature behind a flag. While this isn't enormously expensive, I don't want to take the performance hit to verify something I already guaranteed some other way. This is a speed-critical library for many use cases.

// the multi-threaded solver, which can read out of bounds and crash the
// process -- intermittently, depending on the number of tables and heap
// layout -- before the per-board card-count check fires. Reject up front so
// batch calls fail cleanly with RETURN_CARD_COUNT, matching the contract the
// single-board solver already enforces.
for (int m = 0; m < dealsp->no_of_tables; m++)
for (int h = 0; h < DDS_HANDS; h++)
{
int cardsInHand = 0;
for (int s = 0; s < DDS_SUITS; s++)
cardsInHand += std::popcount(dealsp->deals[m].cards[h][s]);
if (cardsInHand != 13)
Comment on lines +267 to +270
return RETURN_CARD_COUNT;
}

int ind = 0;
int lastIndex = 0;
resp->no_of_boards = 0;
Expand Down
20 changes: 20 additions & 0 deletions python/tests/test_calc_tables_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,25 @@ def test_nt_row_index_regression(self) -> None:
self.assertEqual(len(all_rows), 5)
self.assertEqual(len(nt_only_rows), 5)
self.assertEqual(nt_only_rows[4], all_rows[4])

def test_malformed_deal_in_batch_raises_not_crashes(self) -> None:
"""A malformed deal (a hand without 13 cards) must raise cleanly, even
inside a multi-deal batch. Regression: such a deal previously reached
the multi-threaded solver and segfaulted the process (intermittently,
depending on batch size) instead of returning RETURN_CARD_COUNT (-14).
"""
valid = "N:976.2.QJ74.JT985 83.JT53.K.AQ7643 QJ4.AQ984.T65.K2 AKT52.K76.A9832.-"
# North holds only 12 cards (empty clubs): A5.QJ953.KQ962.(void)
malformed = "N:A5.QJ953.KQ962. KJT8.K74..AJ62 632.A6.T74.KQ953 Q974.T82.AJ5.T74"

# Solo: already rejected by the single-board card-count check.
with self.assertRaises(RuntimeError):
calc_all_tables_pbn([malformed])
Comment on lines +35 to +36

# In a batch: must also raise, not crash the interpreter.
with self.assertRaises(RuntimeError):
calc_all_tables_pbn([valid] * 20 + [malformed])
Comment on lines +39 to +40


if __name__ == "__main__":
unittest.main()
Loading