Skip to content

logging: log output busy loops if log output function can not process buffer #12241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
b0661 opened this issue Dec 29, 2018 · 3 comments
Closed
Assignees
Labels
area: Logging bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@b0661
Copy link
Collaborator

b0661 commented Dec 29, 2018

Describe the bug
According to the documentation the log_output_func shall return the number of bytes processed.

typedef int (*log_output_func_t)(u8_t *buf, size_t size, void *ctx)

    Prototype of the function processing output data.

    Return
        Number of bytes processed. 
    Parameters

            data: Data.
            length: Data length.
            ctx: User context.

If the function returns 0 the log buffer_write function busy loops forever.

static void buffer_write(log_output_func_t outf, u8_t *buf, size_t len,
			 void *ctx)
{
	int processed;

	do {
		processed = outf(buf, len, ctx);
		len -= processed;
		buf += processed;
	} while (len);
}

To Reproduce
Steps to reproduce the behavior:
Change the log_output_func to return 0.

Expected behavior
Ether - The logger shall stop the output to the backend and come back later.
Or - Improve documentation to reflect the needed behaviour (which is still unclear to me)

Impact
Application stalls if log output stalls.

Screenshots or console output
N/A

Environment (please complete the following information):

Additional context
N/A

@b0661 b0661 added the bug The issue is a bug, or the PR is fixing a bug label Dec 29, 2018
@nordic-krch
Copy link
Collaborator

@b0661 processed means dropped as well. It is backend responsibility to drop data. I would prefer to keep log_output module simple. It's just a helper module for backends to avoid code repetition. I will add a note to api documentation, ok?

@b0661
Copy link
Collaborator Author

b0661 commented Jan 7, 2019

@nordic-krch

It's just a helper module for backends to avoid code repetition. I will add a note to api documentation, ok?

Can you just make it typedef void (*log_output_func_t)(u8_t *buf, size_t size, void *ctx)?

By this it wouid be clear by interface that the whole buffer has to be handled by the backend. You may get an extra loop in the backend but could get rid of e.g. the buffer_write helper.

Otherwise a note in the api documentation is also fine for me.

@nashif nashif added the priority: medium Medium impact/importance bug label Jan 10, 2019
@carlescufi
Copy link
Member

@nordic-krch can you submit a PR for the note in the doc that you agreed to?

carlescufi added a commit to carlescufi/zephyr that referenced this issue Mar 26, 2019
The log_output_func_t backend function is supposed to either process or
drop bytes and return the amount of those to the caller. Clarify this in
the documentation.

Fixes zephyrproject-rtos#12241

Signed-off-by: Carles Cufi <[email protected]>
carlescufi added a commit that referenced this issue Mar 27, 2019
The log_output_func_t backend function is supposed to either process or
drop bytes and return the amount of those to the caller. Clarify this in
the documentation.

Fixes #12241

Signed-off-by: Carles Cufi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Logging bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

4 participants