Skip to content

Commit c064ffb

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 cd054aa commit c064ffb

File tree

1 file changed

+29
-15
lines changed

1 file changed

+29
-15
lines changed

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

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,23 +1276,34 @@ ggml_tensor * rpc_server::create_node(uint64_t id,
12761276
}
12771277
tensor_map[id] = result;
12781278
for (int i = 0; i < GGML_MAX_SRC; i++) {
1279-
result->src[i] = create_node(tensor->src[i], ctx, tensor_ptrs, tensor_map);
1280-
// Check if recursive call failed
1281-
if (tensor->src[i] != 0 && result->src[i] == nullptr) {
1282-
GGML_LOG_ERROR("[%s] failed to create source node %d (src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1283-
__func__, i, tensor->src[i], id);
1279+
// Check if the source ID is 0 before calling create_node recursively
1280+
if (tensor->src[i] == 0) {
1281+
result->src[i] = nullptr;
1282+
} else {
1283+
result->src[i] = create_node(tensor->src[i], ctx, tensor_ptrs, tensor_map);
1284+
// If the recursive call failed for a non-zero ID, propagate the error
1285+
if (result->src[i] == nullptr) {
1286+
GGML_LOG_ERROR("[%s] failed to create source node %d (src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1287+
__func__, i, tensor->src[i], id);
1288+
// Must return nullptr to signal failure up the call stack
1289+
return nullptr;
1290+
}
1291+
}
1292+
}
1293+
1294+
// Handle view_src similarly
1295+
if (tensor->view_src == 0) {
1296+
result->view_src = nullptr;
1297+
} else {
1298+
result->view_src = create_node(tensor->view_src, ctx, tensor_ptrs, tensor_map);
1299+
// If the recursive call failed for a non-zero ID, propagate the error
1300+
if (result->view_src == nullptr) {
1301+
GGML_LOG_ERROR("[%s] failed to create view_src node (view_src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1302+
__func__, tensor->view_src, id);
12841303
// Must return nullptr to signal failure up the call stack
12851304
return nullptr;
12861305
}
12871306
}
1288-
result->view_src = create_node(tensor->view_src, ctx, tensor_ptrs, tensor_map);
1289-
// Check if recursive call failed
1290-
if (tensor->view_src != 0 && result->view_src == nullptr) {
1291-
GGML_LOG_ERROR("[%s] failed to create view_src node (view_src_id=%" PRIu64 ") for node id %" PRIu64 "\n",
1292-
__func__, tensor->view_src, id);
1293-
// Must return nullptr to signal failure up the call stack
1294-
return nullptr;
1295-
}
12961307
result->view_offs = tensor->view_offs;
12971308
return result;
12981309
}
@@ -1391,8 +1402,11 @@ bool rpc_server::graph_compute(const std::vector<uint8_t> & input, rpc_msg_graph
13911402
int64_t id;
13921403
memcpy(&id, &nodes_ptr[i], sizeof(id));
13931404
graph->nodes[i] = create_node(id, ctx, tensor_ptrs, tensor_map);
1394-
// Check if create_node failed (e.g., due to invalid type or missing ID)
1395-
if (graph->nodes[i] == nullptr) {
1405+
1406+
// Check if create_node failed for a *non-zero* ID.
1407+
// If id was 0, create_node returning nullptr is expected.
1408+
// If id was non-zero and create_node returned nullptr, it indicates a deserialization error.
1409+
if (graph->nodes[i] == nullptr && id != 0) {
13961410
GGML_LOG_ERROR("[%s] failed to create graph node %d (id=%" PRId64 ")\n", __func__, i, id);
13971411
response.result = GGML_STATUS_FAILED;
13981412
// No need to free ctx, ggml_context_ptr handles it.

0 commit comments

Comments
 (0)