Commit 08206d59 authored by Scott Vokes's avatar Scott Vokes
Browse files

Extract constants for P2P limits. 

Also, fix a double-GIVE for the counting semaphore caused by incorrect
error handling in the P2POP system test, and add an assert to
kinetic_countingsemaphore to catch similar problems in the future.
parent 7368b67c

include/kinetic_types.h

+10 −0
Original line number Diff line number Diff line
@@ -430,6 +430,16 @@ struct _KineticP2P_Operation { 
    KineticP2P_OperationData* operations; // pointer must remain valid until operations complete

};



/**

 * @brief Limit for P2P operations.

 */

#define KINETIC_P2P_OPERATION_LIMIT 100000



/**

 * @brief Limit for P2P operation nesting.

 */

#define KINETIC_P2P_MAX_NESTING 1000



/**

 * @brief Default values for the KineticClientConfig struct, which will be used

 * if the corresponding field in the struct is 0.

src/lib/kinetic_client.c

+0 −1
Original line number Diff line number Diff line
@@ -324,7 +324,6 @@ KineticStatus KineticClient_P2POperation(KineticSession const * const session, 
        if (closure != NULL) {

            operation->closure = *closure;

        }

        KineticOperation_Complete(operation, status);

        return status;

    }

src/lib/kinetic_countingsemaphore.c

+2 −0
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ KineticCountingSemaphore * KineticCountingSemaphore_Create(uint32_t counts) 
    pthread_mutex_init(&sem->mutex, NULL);

    pthread_cond_init(&sem->available, NULL);

    sem->count = counts;

    sem->max = counts;

    sem->num_waiting = 0;

    return sem;

}
@@ -72,6 +73,7 @@ void KineticCountingSemaphore_Give(KineticCountingSemaphore * const sem) // SIGN 
    pthread_mutex_unlock(&sem->mutex);

    

    LOGF3("Concurrent ops throttle -- GIVE: %u => %u (waiting=%u)", before, after, waiting);

    assert(sem->max >= after);

}



void KineticCountingSemaphore_Destroy(KineticCountingSemaphore * const sem)

src/lib/kinetic_operation.c

+4 −4
Original line number Diff line number Diff line
@@ -588,9 +588,9 @@ void destroy_p2pOp(KineticProto_Command_P2POperation* proto_p2pOp) 


KineticProto_Command_P2POperation* build_p2pOp(uint32_t nestingLevel, KineticP2P_Operation const * const p2pOp)

{

    // limit nesting level to 10000

    if (nestingLevel == 1000) {

        LOG0("P2P operation nesting level is too deep. Max is 1000.");

    // limit nesting level to KINETIC_P2P_MAX_NESTING

    if (nestingLevel >= KINETIC_P2P_MAX_NESTING) {

        LOGF0("P2P operation nesting level is too deep. Max is %d.", KINETIC_P2P_MAX_NESTING);

        return NULL;

    }
@@ -719,7 +719,7 @@ KineticStatus KineticOperation_BuildP2POperation(KineticOperation* const operati 
        return KINETIC_STATUS_OPERATION_INVALID;

    }



    if (p2pOp->numOperations >= 100000) {

    if (p2pOp->numOperations >= KINETIC_P2P_OPERATION_LIMIT) {

        return KINETIC_STATUS_BUFFER_OVERRUN;

    }

test/system/test_system_p2pop.c

+11 −13
Original line number Diff line number Diff line
@@ -422,12 +422,11 @@ void disabled_test_P2P_should_support_nesting_of_p2p_operations(void) 
    TEST_ASSERT_EQUAL_MESSAGE(KINETIC_STATUS_SUCCESS, status, "Error when disconnecting client!");

}



void test_P2P_should_fail_with_a_buffer_overrun_error_if_to_many_operations_specified(void)

void test_P2P_should_fail_with_a_buffer_overrun_error_if_too_many_operations_specified(void)

{



    size_t to_many_operations = 100000;

    KineticP2P_OperationData * ops = calloc(to_many_operations, sizeof(KineticP2P_OperationData));

    for (size_t i = 0; i < to_many_operations; i++) {

    size_t too_many_operations = KINETIC_P2P_OPERATION_LIMIT;

    KineticP2P_OperationData * ops = calloc(too_many_operations, sizeof(KineticP2P_OperationData));

    for (size_t i = 0; i < too_many_operations; i++) {

        ops[i] = (KineticP2P_OperationData){

            .key = Key1Buffer,

            .newKey = Key3Buffer,
@@ -439,7 +438,7 @@ void test_P2P_should_fail_with_a_buffer_overrun_error_if_to_many_operations_spec 
                  .port = KINETIC_TEST_PORT1,

                  .tls = false,

                },

        .numOperations = to_many_operations,

        .numOperations = too_many_operations,

        .operations = ops

    };
@@ -450,18 +449,17 @@ void test_P2P_should_fail_with_a_buffer_overrun_error_if_to_many_operations_spec 
}





void test_P2P_should_fail_with_a_operation_invalid_if_to_many_chained_p2p_operations(void)

void test_P2P_should_fail_with_a_operation_invalid_if_too_many_chained_p2p_operations(void)

{

    size_t too_many_operations = KINETIC_P2P_OPERATION_LIMIT + 1;

    KineticP2P_OperationData * ops = calloc(too_many_operations, sizeof(KineticP2P_OperationData));

    KineticP2P_Operation * chained_ops = calloc(too_many_operations, sizeof(KineticP2P_Operation));



    size_t to_many_operations = 1001;

    KineticP2P_OperationData * ops = calloc(to_many_operations, sizeof(KineticP2P_OperationData));

    KineticP2P_Operation * chained_ops = calloc(to_many_operations, sizeof(KineticP2P_Operation));



    for (size_t i = 0; i < to_many_operations; i++) {

    for (size_t i = 0; i < too_many_operations; i++) {

        ops[i] = (KineticP2P_OperationData){

            .key = Key1Buffer,

            .newKey = Key3Buffer,

            .chainedOperation = (i == (to_many_operations - 1)) ? NULL : &chained_ops[i + 1],

            .chainedOperation = (i == (too_many_operations - 1)) ? NULL : &chained_ops[i + 1],

        };

        chained_ops[i] = (KineticP2P_Operation){

            .peer = { .hostname = KINETIC_TEST_HOST1,