Skip to content

Commit 604f0a0

Browse files
committed
refactor(rpc): ambiguous nullptr from create_node
rpc_server::create_node could previously return nullptr if the input ID was 0 (valid) or if an internal error (deserialization, recursion failure) occurred (invalid). This ambiguity made error handling difficult for the caller (`graph_compute`). This commit clarifies the meaning of nullptr: - `graph_compute` now checks if the input 'id' was non-zero when `create_node` returns nullptr, correctly identifying failures versus intentional null links. - `create_node` avoids recursive calls for zero IDs and propagates nullptr unambiguously on failure during recursion. Signed-off-by: Ville Vesilehto <[email protected]>
1 parent 07c0518 commit 604f0a0

File tree

1 file changed

+29
-15
lines changed

1 file changed

+29
-15
lines changed

ggml/src/ggml-rpc/ggml-rpc.cpp

+29-15
Original file line numberDiff line numberDiff line change
@@ -1267,23 +1267,34 @@ ggml_tensor * rpc_server::create_node(uint64_t id,
12671267
}
12681268
tensor_map[id] = result;
12691269
for (int i = 0; i < GGML_MAX_SRC; i++) {
1270-
result->src[i] = create_node(tensor->src[i], ctx, tensor_ptrs, tensor_map);
1271-
// Check if recursive call failed
1272-
if (tensor->src[i] != 0 && result->src[i] == nullptr) {
1273-
GGML_LOG_ERROR("[%s] failed to create source node %d (src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1274-
__func__, i, tensor->src[i], id);
1270+
// Check if the source ID is 0 before calling create_node recursively
1271+
if (tensor->src[i] == 0) {
1272+
result->src[i] = nullptr;
1273+
} else {
1274+
result->src[i] = create_node(tensor->src[i], ctx, tensor_ptrs, tensor_map);
1275+
// If the recursive call failed for a non-zero ID, propagate the error
1276+
if (result->src[i] == nullptr) {
1277+
GGML_LOG_ERROR("[%s] failed to create source node %d (src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1278+
__func__, i, tensor->src[i], id);
1279+
// Must return nullptr to signal failure up the call stack
1280+
return nullptr;
1281+
}
1282+
}
1283+
}
1284+
1285+
// Handle view_src similarly
1286+
if (tensor->view_src == 0) {
1287+
result->view_src = nullptr;
1288+
} else {
1289+
result->view_src = create_node(tensor->view_src, ctx, tensor_ptrs, tensor_map);
1290+
// If the recursive call failed for a non-zero ID, propagate the error
1291+
if (result->view_src == nullptr) {
1292+
GGML_LOG_ERROR("[%s] failed to create view_src node (view_src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1293+
__func__, tensor->view_src, id);
12751294
// Must return nullptr to signal failure up the call stack
12761295
return nullptr;
12771296
}
12781297
}
1279-
result->view_src = create_node(tensor->view_src, ctx, tensor_ptrs, tensor_map);
1280-
// Check if recursive call failed
1281-
if (tensor->view_src != 0 && result->view_src == nullptr) {
1282-
GGML_LOG_ERROR("[%s] failed to create view_src node (view_src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1283-
__func__, tensor->view_src, id);
1284-
// Must return nullptr to signal failure up the call stack
1285-
return nullptr;
1286-
}
12871298
result->view_offs = tensor->view_offs;
12881299
return result;
12891300
}
@@ -1382,8 +1393,11 @@ bool rpc_server::graph_compute(const std::vector<uint8_t> & input, rpc_msg_graph
13821393
int64_t id;
13831394
memcpy(&id, &nodes_ptr[i], sizeof(id));
13841395
graph->nodes[i] = create_node(id, ctx, tensor_ptrs, tensor_map);
1385-
// Check if create_node failed (e.g., due to invalid type or missing ID)
1386-
if (graph->nodes[i] == nullptr) {
1396+
1397+
// Check if create_node failed for a *non-zero* ID.
1398+
// If id was 0, create_node returning nullptr is expected.
1399+
// If id was non-zero and create_node returned nullptr, it indicates a deserialization error.
1400+
if (graph->nodes[i] == nullptr && id != 0) {
13871401
GGML_LOG_ERROR("[%s] failed to create graph node %d (id=%" PRId64 ")\n", __func__, i, id);
13881402
response.result = GGML_STATUS_FAILED;
13891403
// No need to free ctx, ggml_context_ptr handles it.

0 commit comments

Comments
 (0)