Skip to content

Commit

Permalink
Merge pull request grpc#2972 from ctiller/get-reserved-things-right
Browse files Browse the repository at this point in the history
Add some reserved checks that need to be present
  • Loading branch information
yang-g committed Aug 19, 2015
2 parents 5068fa4 + b815fb2 commit 4836492
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/core/surface/call.c
Original file line number Diff line number Diff line change
Expand Up @@ -1573,7 +1573,8 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops,
const grpc_op *op;
grpc_ioreq *req;
void (*finish_func)(grpc_call *, int, void *) = finish_batch;
GPR_ASSERT(!reserved);

if (reserved != NULL) return GRPC_CALL_ERROR;

GRPC_CALL_LOG_BATCH(GPR_INFO, call, ops, nops, tag);

Expand All @@ -1588,6 +1589,7 @@ grpc_call_error grpc_call_start_batch(grpc_call *call, const grpc_op *ops,
/* rewrite batch ops into ioreq ops */
for (in = 0, out = 0; in < nops; in++) {
op = &ops[in];
if (op->reserved != NULL) return GRPC_CALL_ERROR;
switch (op->op) {
case GRPC_OP_SEND_INITIAL_METADATA:
/* Flag validation: currently allow no flags */
Expand Down
1 change: 1 addition & 0 deletions src/core/surface/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,7 @@ grpc_call_error grpc_server_request_call(
return GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE;
}
grpc_cq_begin_op(cq_for_notification);
details->reserved = NULL;
rc->type = BATCH_CALL;
rc->server = server;
rc->tag = tag;
Expand Down
26 changes: 26 additions & 0 deletions src/csharp/ext/grpc_csharp_ext.c
Original file line number Diff line number Diff line change
Expand Up @@ -510,22 +510,27 @@ grpcsharp_call_start_unary(grpc_call *call, grpcsharp_batch_context *ctx,
ops[0].data.send_initial_metadata.metadata =
ctx->send_initial_metadata.metadata;
ops[0].flags = 0;
ops[0].reserved = NULL;

ops[1].op = GRPC_OP_SEND_MESSAGE;
ctx->send_message = string_to_byte_buffer(send_buffer, send_buffer_len);
ops[1].data.send_message = ctx->send_message;
ops[1].flags = write_flags;
ops[1].reserved = NULL;

ops[2].op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
ops[2].flags = 0;
ops[2].reserved = NULL;

ops[3].op = GRPC_OP_RECV_INITIAL_METADATA;
ops[3].data.recv_initial_metadata = &(ctx->recv_initial_metadata);
ops[3].flags = 0;
ops[3].reserved = NULL;

ops[4].op = GRPC_OP_RECV_MESSAGE;
ops[4].data.recv_message = &(ctx->recv_message);
ops[4].flags = 0;
ops[4].reserved = NULL;

ops[5].op = GRPC_OP_RECV_STATUS_ON_CLIENT;
ops[5].data.recv_status_on_client.trailing_metadata =
Expand All @@ -538,6 +543,7 @@ grpcsharp_call_start_unary(grpc_call *call, grpcsharp_batch_context *ctx,
ops[5].data.recv_status_on_client.status_details_capacity =
&(ctx->recv_status_on_client.status_details_capacity);
ops[5].flags = 0;
ops[5].reserved = NULL;

return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
Expand All @@ -556,14 +562,17 @@ grpcsharp_call_start_client_streaming(grpc_call *call,
ops[0].data.send_initial_metadata.metadata =
ctx->send_initial_metadata.metadata;
ops[0].flags = 0;
ops[0].reserved = NULL;

ops[1].op = GRPC_OP_RECV_INITIAL_METADATA;
ops[1].data.recv_initial_metadata = &(ctx->recv_initial_metadata);
ops[1].flags = 0;
ops[1].reserved = NULL;

ops[2].op = GRPC_OP_RECV_MESSAGE;
ops[2].data.recv_message = &(ctx->recv_message);
ops[2].flags = 0;
ops[2].reserved = NULL;

ops[3].op = GRPC_OP_RECV_STATUS_ON_CLIENT;
ops[3].data.recv_status_on_client.trailing_metadata =
Expand All @@ -576,6 +585,7 @@ grpcsharp_call_start_client_streaming(grpc_call *call,
ops[3].data.recv_status_on_client.status_details_capacity =
&(ctx->recv_status_on_client.status_details_capacity);
ops[3].flags = 0;
ops[3].reserved = NULL;

return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
Expand All @@ -593,18 +603,22 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_server_streaming(
ops[0].data.send_initial_metadata.metadata =
ctx->send_initial_metadata.metadata;
ops[0].flags = 0;
ops[0].reserved = NULL;

ops[1].op = GRPC_OP_SEND_MESSAGE;
ctx->send_message = string_to_byte_buffer(send_buffer, send_buffer_len);
ops[1].data.send_message = ctx->send_message;
ops[1].flags = write_flags;
ops[1].reserved = NULL;

ops[2].op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
ops[2].flags = 0;
ops[2].reserved = NULL;

ops[3].op = GRPC_OP_RECV_INITIAL_METADATA;
ops[3].data.recv_initial_metadata = &(ctx->recv_initial_metadata);
ops[3].flags = 0;
ops[3].reserved = NULL;

ops[4].op = GRPC_OP_RECV_STATUS_ON_CLIENT;
ops[4].data.recv_status_on_client.trailing_metadata =
Expand All @@ -617,6 +631,7 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_server_streaming(
ops[4].data.recv_status_on_client.status_details_capacity =
&(ctx->recv_status_on_client.status_details_capacity);
ops[4].flags = 0;
ops[4].reserved = NULL;

return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
Expand All @@ -635,10 +650,12 @@ grpcsharp_call_start_duplex_streaming(grpc_call *call,
ops[0].data.send_initial_metadata.metadata =
ctx->send_initial_metadata.metadata;
ops[0].flags = 0;
ops[0].reserved = NULL;

ops[1].op = GRPC_OP_RECV_INITIAL_METADATA;
ops[1].data.recv_initial_metadata = &(ctx->recv_initial_metadata);
ops[1].flags = 0;
ops[1].reserved = NULL;

ops[2].op = GRPC_OP_RECV_STATUS_ON_CLIENT;
ops[2].data.recv_status_on_client.trailing_metadata =
Expand All @@ -651,6 +668,7 @@ grpcsharp_call_start_duplex_streaming(grpc_call *call,
ops[2].data.recv_status_on_client.status_details_capacity =
&(ctx->recv_status_on_client.status_details_capacity);
ops[2].flags = 0;
ops[2].reserved = NULL;

return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
Expand All @@ -668,10 +686,12 @@ grpcsharp_call_send_message(grpc_call *call, grpcsharp_batch_context *ctx,
ctx->send_message = string_to_byte_buffer(send_buffer, send_buffer_len);
ops[0].data.send_message = ctx->send_message;
ops[0].flags = write_flags;
ops[0].reserved = NULL;
ops[1].op = GRPC_OP_SEND_INITIAL_METADATA;
ops[1].data.send_initial_metadata.count = 0;
ops[1].data.send_initial_metadata.metadata = NULL;
ops[1].flags = 0;
ops[1].reserved = NULL;

return grpc_call_start_batch(call, ops, nops, ctx, NULL);
}
Expand All @@ -683,6 +703,7 @@ grpcsharp_call_send_close_from_client(grpc_call *call,
grpc_op ops[1];
ops[0].op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
ops[0].flags = 0;
ops[0].reserved = NULL;

return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
Expand All @@ -706,10 +727,12 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_send_status_from_server(
ops[0].data.send_status_from_server.trailing_metadata =
ctx->send_status_from_server.trailing_metadata.metadata;
ops[0].flags = 0;
ops[0].reserved = NULL;
ops[1].op = GRPC_OP_SEND_INITIAL_METADATA;
ops[1].data.send_initial_metadata.count = 0;
ops[1].data.send_initial_metadata.metadata = NULL;
ops[1].flags = 0;
ops[1].reserved = NULL;

return grpc_call_start_batch(call, ops, nops, ctx, NULL);
}
Expand All @@ -721,6 +744,7 @@ grpcsharp_call_recv_message(grpc_call *call, grpcsharp_batch_context *ctx) {
ops[0].op = GRPC_OP_RECV_MESSAGE;
ops[0].data.recv_message = &(ctx->recv_message);
ops[0].flags = 0;
ops[0].reserved = NULL;
return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
}
Expand All @@ -733,6 +757,7 @@ grpcsharp_call_start_serverside(grpc_call *call, grpcsharp_batch_context *ctx) {
ops[0].data.recv_close_on_server.cancelled =
(&ctx->recv_close_on_server_cancelled);
ops[0].flags = 0;
ops[0].reserved = NULL;

return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
Expand All @@ -751,6 +776,7 @@ grpcsharp_call_send_initial_metadata(grpc_call *call,
ops[0].data.send_initial_metadata.metadata =
ctx->send_initial_metadata.metadata;
ops[0].flags = 0;
ops[0].reserved = NULL;

return grpc_call_start_batch(call, ops, sizeof(ops) / sizeof(ops[0]), ctx,
NULL);
Expand Down
1 change: 1 addition & 0 deletions src/node/ext/call.cc
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ NAN_METHOD(Call::StartBatch) {
uint32_t type = keys->Get(i)->Uint32Value();
ops[i].op = static_cast<grpc_op_type>(type);
ops[i].flags = 0;
ops[i].reserved = NULL;
switch (type) {
case GRPC_OP_SEND_INITIAL_METADATA:
op.reset(new SendMetadataOp());
Expand Down
1 change: 1 addition & 0 deletions src/php/ext/grpc/call.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ PHP_METHOD(Call, startBatch) {
}
ops[op_num].op = (grpc_op_type)index;
ops[op_num].flags = 0;
ops[op_num].reserved = NULL;
op_num++;
}
error = grpc_call_start_batch(call->wrapped, ops, op_num, call->wrapped,
Expand Down
1 change: 1 addition & 0 deletions src/python/grpcio/grpc/_adapter/_c/utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ int pygrpc_produce_op(PyObject *op, grpc_op *result) {
return 0;
}
c_op.op = type;
c_op.reserved = NULL;
c_op.flags = PyInt_AsLong(PyTuple_GET_ITEM(op, WRITE_FLAGS_INDEX));
if (PyErr_Occurred()) {
return 0;
Expand Down
1 change: 1 addition & 0 deletions src/ruby/ext/grpc/rb_call.c
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,7 @@ static void grpc_run_batch_stack_fill_ops(run_batch_stack *st, VALUE ops_hash) {
};
st->ops[st->op_num].op = (grpc_op_type)NUM2INT(this_op);
st->ops[st->op_num].flags = 0;
st->ops[st->op_num].reserved = NULL;
st->op_num++;
}
}
Expand Down
8 changes: 8 additions & 0 deletions test/core/end2end/fixtures/proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ static void on_p2s_recv_initial_metadata(void *arg, int success) {
if (!pc->proxy->shutdown) {
op.op = GRPC_OP_SEND_INITIAL_METADATA;
op.flags = 0;
op.reserved = NULL;
op.data.send_initial_metadata.count = pc->p2s_initial_metadata.count;
op.data.send_initial_metadata.metadata = pc->p2s_initial_metadata.metadata;
refpc(pc, "on_c2p_sent_initial_metadata");
Expand Down Expand Up @@ -201,6 +202,7 @@ static void on_p2s_sent_message(void *arg, int success) {
if (!pc->proxy->shutdown && success) {
op.op = GRPC_OP_RECV_MESSAGE;
op.flags = 0;
op.reserved = NULL;
op.data.recv_message = &pc->c2p_msg;
refpc(pc, "on_c2p_recv_msg");
err = grpc_call_start_batch(pc->c2p, &op, 1,
Expand All @@ -225,6 +227,7 @@ static void on_c2p_recv_msg(void *arg, int success) {
if (pc->c2p_msg != NULL) {
op.op = GRPC_OP_SEND_MESSAGE;
op.flags = 0;
op.reserved = NULL;
op.data.send_message = pc->c2p_msg;
refpc(pc, "on_p2s_sent_message");
err = grpc_call_start_batch(pc->p2s, &op, 1,
Expand All @@ -233,6 +236,7 @@ static void on_c2p_recv_msg(void *arg, int success) {
} else {
op.op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
op.flags = 0;
op.reserved = NULL;
refpc(pc, "on_p2s_sent_close");
err = grpc_call_start_batch(pc->p2s, &op, 1,
new_closure(on_p2s_sent_close, pc), NULL);
Expand All @@ -254,6 +258,7 @@ static void on_c2p_sent_message(void *arg, int success) {
if (!pc->proxy->shutdown && success) {
op.op = GRPC_OP_RECV_MESSAGE;
op.flags = 0;
op.reserved = NULL;
op.data.recv_message = &pc->p2s_msg;
refpc(pc, "on_p2s_recv_msg");
err = grpc_call_start_batch(pc->p2s, &op, 1,
Expand All @@ -272,6 +277,7 @@ static void on_p2s_recv_msg(void *arg, int success) {
if (!pc->proxy->shutdown && success && pc->p2s_msg) {
op.op = GRPC_OP_SEND_MESSAGE;
op.flags = 0;
op.reserved = NULL;
op.data.send_message = pc->p2s_msg;
refpc(pc, "on_c2p_sent_message");
err = grpc_call_start_batch(pc->c2p, &op, 1,
Expand All @@ -295,6 +301,7 @@ static void on_p2s_status(void *arg, int success) {
GPR_ASSERT(success);
op.op = GRPC_OP_SEND_STATUS_FROM_SERVER;
op.flags = 0;
op.reserved = NULL;
op.data.send_status_from_server.trailing_metadata_count =
pc->p2s_trailing_metadata.count;
op.data.send_status_from_server.trailing_metadata =
Expand Down Expand Up @@ -334,6 +341,7 @@ static void on_new_call(void *arg, int success) {
gpr_ref_init(&pc->refs, 1);

op.flags = 0;
op.reserved = NULL;

op.op = GRPC_OP_RECV_INITIAL_METADATA;
op.data.recv_initial_metadata = &pc->p2s_initial_metadata;
Expand Down
7 changes: 7 additions & 0 deletions test/core/end2end/tests/default_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,20 +135,24 @@ static void simple_request_body(grpc_end2end_test_fixture f) {
op->op = GRPC_OP_SEND_INITIAL_METADATA;
op->data.send_initial_metadata.count = 0;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_RECV_INITIAL_METADATA;
op->data.recv_initial_metadata = &initial_metadata_recv;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_RECV_STATUS_ON_CLIENT;
op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv;
op->data.recv_status_on_client.status = &status;
op->data.recv_status_on_client.status_details = &details;
op->data.recv_status_on_client.status_details_capacity = &details_capacity;
op->flags = 0;
op->reserved = NULL;
op++;
error = grpc_call_start_batch(c, ops, op - ops, tag(1), NULL);
GPR_ASSERT(error == GRPC_CALL_OK);
Expand All @@ -173,16 +177,19 @@ static void simple_request_body(grpc_end2end_test_fixture f) {
op->op = GRPC_OP_SEND_INITIAL_METADATA;
op->data.send_initial_metadata.count = 0;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_SEND_STATUS_FROM_SERVER;
op->data.send_status_from_server.trailing_metadata_count = 0;
op->data.send_status_from_server.status = GRPC_STATUS_UNIMPLEMENTED;
op->data.send_status_from_server.status_details = "xyz";
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_RECV_CLOSE_ON_SERVER;
op->data.recv_close_on_server.cancelled = &was_cancelled;
op->flags = 0;
op->reserved = NULL;
op++;
error = grpc_call_start_batch(s, ops, op - ops, tag(102), NULL);
GPR_ASSERT(error == GRPC_CALL_OK);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,25 +438,31 @@ static void test_request_with_server_rejecting_client_creds(
op->data.recv_status_on_client.status_details = &details;
op->data.recv_status_on_client.status_details_capacity = &details_capacity;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_SEND_INITIAL_METADATA;
op->data.send_initial_metadata.count = 0;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_SEND_MESSAGE;
op->data.send_message = request_payload;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_RECV_INITIAL_METADATA;
op->data.recv_initial_metadata = &initial_metadata_recv;
op->flags = 0;
op->reserved = NULL;
op++;
op->op = GRPC_OP_RECV_MESSAGE;
op->data.recv_message = &response_payload_recv;
op->flags = 0;
op->reserved = NULL;
op++;
error = grpc_call_start_batch(c, ops, op - ops, tag(1), NULL);
GPR_ASSERT(error == GRPC_CALL_OK);
Expand Down

0 comments on commit 4836492

Please sign in to comment.