Update C chat samples with using ChatHistory class #3045

zhaohb · 2025-11-19T05:35:23Z

No description provided.

src/c/include/openvino/genai/c/chat_history.h

src/c/src/chat_history.cpp

src/c/include/openvino/genai/c/chat_history.h

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-03T09:09:10Z

samples/c/text_generation/chat_sample_c.c

+        if (message_container) {
+            ov_genai_json_container_free(message_container);
+            message_container = NULL;
+        }
+        CHECK_JSON_CONTAINER_STATUS(ov_genai_json_container_create_from_json_string(
+            message_json, &message_container));
+
+        // Push message using JsonContainer
+        CHECK_CHAT_HISTORY_STATUS(ov_genai_chat_history_push_back(chat_history, message_container));
+
+        results = NULL;
+        CHECK_STATUS(ov_genai_llm_pipeline_generate_with_history(pipeline,
+                                                                  chat_history,
+                                                                  config,
+                                                                  &streamer,
+                                                                  &results));
+
+        if (results) {
+            output_size = sizeof(output_buffer);
+            CHECK_STATUS(ov_genai_decoded_results_get_string(results, output_buffer, &output_size));
+
+            json_escape_string(output_buffer, escaped_output, sizeof(escaped_output));
+
+            snprintf(assistant_message_json, sizeof(assistant_message_json),
+                     "{\"role\": \"assistant\", \"content\": \"%s\"}", escaped_output);
+
+            if (assistant_message_container) {
+                ov_genai_json_container_free(assistant_message_container);
+                assistant_message_container = NULL;
+            }
+            CHECK_JSON_CONTAINER_STATUS(ov_genai_json_container_create_from_json_string(
+                assistant_message_json, &assistant_message_container));
+
+            // Push message using JsonContainer
+            CHECK_CHAT_HISTORY_STATUS(ov_genai_chat_history_push_back(chat_history, assistant_message_container));


Inefficient memory management: the code creates and frees message_container and assistant_message_container in every iteration of the loop (lines 135-138 and 161-164). Since these containers are only used temporarily to wrap JSON strings before pushing to history, consider reusing them across iterations or freeing them immediately after push_back succeeds rather than keeping them alive until the next iteration.

Copilot · 2025-12-03T09:09:11Z

samples/c/text_generation/chat_sample_c.c

+static void json_escape_string(const char* input, char* output, size_t output_size) {
+    size_t i = 0;
+    size_t j = 0;
+    while (input[i] != '\0' && j < output_size - 1) {
+        switch (input[i]) {
+            case '"':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = '"';
+                }
+                break;
+            case '\\':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = '\\';
+                }
+                break;
+            case '\n':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = 'n';
+                }
+                break;
+            case '\r':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = 'r';
+                }
+                break;
+            case '\t':
+                if (j < output_size - 2) {
+                    output[j++] = '\\';
+                    output[j++] = 't';
+                }
+                break;
+            default:
+                output[j++] = input[i];
+                break;
+        }
+        i++;
+    }
+    output[j] = '\0';
+}


The json_escape_string function has a buffer overflow vulnerability. When escaping characters like ", \, \n, \r, and \t, the code checks j < output_size - 2 before writing two characters. However, if the original input fills most of the buffer with characters that need escaping, the output could exceed output_size. The function should track the required output size and return an error if the buffer is insufficient, or ensure proper bounds checking before writing any escaped sequence.

Copilot · 2025-12-03T09:09:11Z

src/c/src/json_container.cpp

+        if (!output) {
+            *output_size = json_str.length() + 1;
+        } else {
+            if (*output_size < json_str.length() + 1) {


Using std::strncpy is safer, but the implementation still has a subtle issue. When *output_size < json_str.length() + 1, the function returns OUT_OF_BOUNDS, but it should ideally still set *output_size to the required size so the caller knows how much space is needed. This would be consistent with the behavior when output is NULL (line 89).

Suggested change

if (*output_size < json_str.length() + 1) {

if (*output_size < json_str.length() + 1) {

*output_size = json_str.length() + 1;

Copilot · 2025-12-03T09:09:11Z

src/c/src/chat_history.cpp

+    } catch (const std::exception& e) {
+        return OV_GENAI_CHAT_HISTORY_ERROR;


The function catches const std::exception& e but doesn't use the exception object e. Either log the error message from the exception or remove the variable name to indicate it's intentionally unused (use catch (const std::exception&) instead). This pattern appears throughout the file.

Copilot · 2025-12-03T09:09:12Z

samples/c/text_generation/chat_sample_c.c

+#define CHECK_CHAT_HISTORY_STATUS(return_status)                                                      \
+    if (return_status != OV_GENAI_CHAT_HISTORY_OK) {                                                  \
+        fprintf(stderr, "[ERROR] chat history status %d, line %d\n", return_status, __LINE__);         \
+        goto err;                                                                                      \
+    }
+
+#define CHECK_JSON_CONTAINER_STATUS(return_status)                                                     \
+    if (return_status != OV_GENAI_JSON_CONTAINER_OK) {                                                 \
+        fprintf(stderr, "[ERROR] json container status %d, line %d\n", return_status, __LINE__);        \
+        goto err;                                                                                      \
+    }


[nitpick] The macro names could be more concise. CHECK_CHAT_HISTORY_STATUS and CHECK_JSON_CONTAINER_STATUS are quite long. Consider using shorter names like CHECK_HISTORY_STATUS and CHECK_JSON_STATUS to improve code readability, especially given that these macros are used frequently throughout the code.

Copilot · 2025-12-03T09:09:12Z

samples/c/text_generation/chat_sample_c.c

+
+        if (results) {
+            output_size = sizeof(output_buffer);
+            CHECK_STATUS(ov_genai_decoded_results_get_string(results, output_buffer, &output_size));


Inconsistent error handling: the code uses CHECK_STATUS for pipeline operations but doesn't check the return value of ov_genai_decoded_results_get_string at line 154, which could fail. If it fails, output_buffer would contain uninitialized or stale data, leading to incorrect behavior. The return status should be checked using CHECK_STATUS.

Copilot · 2025-12-03T09:09:12Z

src/c/src/chat_history.cpp

+    } catch (const ov::Exception& e) {
+        return OV_GENAI_CHAT_HISTORY_INVALID_JSON;


Catching const ov::Exception& e but not using the exception object e. Either log the error message or remove the variable name to indicate it's intentionally unused (use catch (const ov::Exception&) instead). This pattern appears multiple times in this file (lines 65, 84, 128, 152, 176, 243, 283).

Update C chat samples with using ChatHistory class

5a41c21

Copilot AI review requested due to automatic review settings November 19, 2025 05:35

github-actions bot added category: LLM samples GenAI LLM samples category: C API labels Nov 19, 2025

Copilot started reviewing on behalf of zhaohb November 19, 2025 05:36 View session

Copilot finished reviewing on behalf of zhaohb November 19, 2025 05:37

This comment was marked as duplicate.

Sign in to view

update

c28ec23

Wovchena requested a review from yatarkan November 19, 2025 07:57

Merge branch 'master' into chat_history_c_api

f114d11

Copilot AI review requested due to automatic review settings November 19, 2025 10:16

Copilot started reviewing on behalf of zhaohb November 19, 2025 10:17 View session

Copilot finished reviewing on behalf of zhaohb November 19, 2025 10:19

This comment was marked as duplicate.

Sign in to view

yatarkan requested changes Nov 19, 2025

View reviewed changes

zhaohb added 2 commits December 1, 2025 10:23

Merge branch 'master' into chat_history_c_api

04089cc

use JsonContainer

bbfceac

Copilot AI review requested due to automatic review settings December 3, 2025 08:40

Copilot started reviewing on behalf of zhaohb December 3, 2025 08:41 View session

Copilot finished reviewing on behalf of zhaohb December 3, 2025 08:43

Simplify the logic for catching errors

0dc1226

This comment was marked as duplicate.

Sign in to view

zhaohb requested a review from yatarkan December 3, 2025 08:50

Apply suggestion

ce474f7

Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings December 3, 2025 09:02

Copilot started reviewing on behalf of zhaohb December 3, 2025 09:03 View session

zhaohb and others added 2 commits December 3, 2025 17:03

Apply suggestion

6948a30

Co-authored-by: Copilot <[email protected]>

Apply suggestion

584652c

Co-authored-by: Copilot <[email protected]>

Copilot finished reviewing on behalf of zhaohb December 3, 2025 09:06

Apply suggestion

74fbb66

Co-authored-by: Copilot <[email protected]>

Copilot AI reviewed Dec 3, 2025

View reviewed changes

	if (*output_size < json_str.length() + 1) {
	if (*output_size < json_str.length() + 1) {
	*output_size = json_str.length() + 1;

		} catch (const std::exception& e) {
		return OV_GENAI_CHAT_HISTORY_ERROR;

		} catch (const ov::Exception& e) {
		return OV_GENAI_CHAT_HISTORY_INVALID_JSON;

Update C chat samples with using ChatHistory class #3045

Are you sure you want to change the base?

Update C chat samples with using ChatHistory class #3045

Conversation

zhaohb commented Nov 19, 2025

Uh oh!

This comment was marked as duplicate.

Uh oh!

This comment was marked as duplicate.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants