Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Common Log (In Progress) #6338

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Common Log (In Progress) #6338

wants to merge 1 commit into from

Conversation

NayanaNagaraj23
Copy link
Contributor

Make changes in Debug log statements to reduce the binary size

gSahitya-samsung
gSahitya-samsung reviewed Aug 14, 2024
View reviewed changes
os/audio/audio.c Outdated Show resolved Hide resolved
@neel-samsung
Copy link
Contributor

Do we need these commits to merge into samsung/TizenRT:master directly? Although some of the commits don't have proper commit message and also description.

sunghan-chang
sunghan-chang reviewed Aug 16, 2024
View reviewed changes
os/binfmt/binfmt_exec.c Outdated Show resolved Hide resolved
os/include/tinyara/common_logs/common_logs.h Outdated Show resolved Hide resolved
os/include/tinyara/common_logs/common_logs.h Outdated Show resolved Hide resolved
os/include/tinyara/common_logs/common_logs.h Outdated
Comment on lines 51 to 52
"Potential memory leak :", //CMN_LOG_MEM_LEAK,
"Buffer overflow detected :", //CMN_LOG_BUFFER_OVERFLOW,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, is it possible to check in code?

os/include/tinyara/common_logs/common_logs.h Outdated
static const char* clog_message_str[CMN_LOG_MSG_MAX] =
{
/* Memory Operations */
"Memory allocation failed :", //CMN_LOG_ALLOC_FAIL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why all has : at the end? Every message has additional message after this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we have messages at some places.

os/include/tinyara/common_logs/common_logs.h Outdated Show resolved Hide resolved
@NayanaNagaraj23 NayanaNagaraj23 changed the title Common Log Common Log (In Progress) Aug 16, 2024
@NayanaNagaraj23 NayanaNagaraj23 force-pushed the temp_log_reduction branch 7 times, most recently from 1deca23 to 1e44cb1 Compare August 26, 2024 07:50
r-prabu
r-prabu requested changes Aug 29, 2024
View reviewed changes
os/audio/audio.c Outdated Show resolved Hide resolved
os/binfmt/binfmt_exec.c Outdated Show resolved Hide resolved
os/binfmt/binfmt_exit.c Outdated Show resolved Hide resolved
os/binfmt/binfmt_initialize.c Outdated Show resolved Hide resolved
os/binfmt/binfmt_initialize.c Outdated Show resolved Hide resolved
os/binfmt/binfmt_loadbinary.c Outdated Show resolved Hide resolved
os/binfmt/binfmt_loadbinary.c Outdated Show resolved Hide resolved
os/binfmt/binfmt_unloadmodule.c Outdated Show resolved Hide resolved
@NayanaNagaraj23 NayanaNagaraj23 force-pushed the temp_log_reduction branch 3 times, most recently from 3ef52c8 to 76acdf3 Compare September 2, 2024 09:23
neel-samsung
neel-samsung reviewed Sep 2, 2024
View reviewed changes
Copy link
Contributor

@neel-samsung neel-samsung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the conflicts and rebase the PR accordingly.

@NayanaNagaraj23 NayanaNagaraj23 force-pushed the temp_log_reduction branch 4 times, most recently from 5d1d435 to 334fb2b Compare September 5, 2024 05:27
@NayanaNagaraj23 NayanaNagaraj23 force-pushed the temp_log_reduction branch 5 times, most recently from e8f0207 to c536712 Compare September 10, 2024 06:29
r-prabu
r-prabu reviewed Sep 11, 2024
View reviewed changes
os/pm/pm_metrics.c Outdated Show resolved Hide resolved
r-prabu
r-prabu reviewed Sep 11, 2024
View reviewed changes
Copy link
Collaborator

@r-prabu r-prabu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit description should have detailed information about the changes & why we did it.

kishore-sn
kishore-sn reviewed Sep 13, 2024
View reviewed changes
os/include/tinyara/common_logs/common_logs.h Outdated
"End of", //CMN_LOG_END,
"Invalid data in", //CMN_LOG_INVALID_DATA,
"Valid data in :" //CMN_LOG_VALID_DATA,
"Not supported", // CMN_LOG_NOT_SUPPORTED
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix indentation of all the above lines.

os/include/tinyara/common_logs/common_logs.h Outdated
static const char* clog_message_str[CMN_LOG_MSG_MAX] =
{
/* Memory Operations */
"Memory allocation failure", //CMN_LOG_ALLOC_FAIL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change failure to failed in all log msgs.

os/include/tinyara/common_logs/common_logs.h Outdated

/* Network Operations */
"Connection Timeout", //CMN_LOG_CON_TIMEOUT,
"Connection Refuse", //CMN_LOG_CON_REFUSED,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refuse -> refused

os/include/tinyara/common_logs/common_logs.h Outdated
"IOCTL call failure", //CMN_LOG_FILE_IOCTL_ERROR,

/* Network Operations */
"Connection Timeout", //CMN_LOG_CON_TIMEOUT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why using capital letter for all words in some log msgs? Pls make it same as other msgs. Use capital letter only for first word.

kishore-sn
kishore-sn reviewed Sep 13, 2024
View reviewed changes
os/drivers/video/video.c
@@ -1300,7 +1301,7 @@ FAR void *video_register(const char *devpath, FAR struct video_lowerhalf_s *dev)

priv = (FAR video_upperhalf_t *)kmm_malloc(sizeof(video_upperhalf_t));
if (!priv) {
videodbg("Failed to allocate instance\n");
videodbg("%s instance\n", clog_message_str[CMN_LOG_ALLOC_FAIL]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

%s for instance

os/drivers/bluetooth/bt_null/bt_null.c
@@ -360,7 +361,7 @@ static int btnull_send(FAR const struct bt_driver_s *dev, FAR struct bt_buf_s *b

outbuf = bt_buf_alloc(BT_EVT, NULL, 0);
if (outbuf == NULL) {
ndbg("ERROR: Failed to allocate buffer\n");
ndbg("%s: bt_buf_alloc\n", clog_message_str[CMN_LOG_ALLOC_FAIL]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove : bt_buf_alloc

os/drivers/lcd/lcd_framebuffer.c
@@ -610,7 +611,7 @@ int up_fbinitialize(int display)

priv->fbmem = (FAR uint8_t *)kmm_zalloc(priv->fblen);
if (priv->fbmem == NULL) {
gdbg("ERROR: Failed to allocate frame buffer memory\n");
gdbg("%s frame buffer\n", clog_message_str[CMN_LOG_ALLOC_FAIL]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for frame buffer

os/drivers/lcd/lcd_framebuffer.c
DEBUGASSERT((unsigned)display < UINT8_MAX);

/* Allocate the framebuffer state structure */

priv = (FAR struct lcdfb_dev_s *)kmm_zalloc(sizeof(struct lcdfb_dev_s));
if (priv == NULL) {
gdbg("ERROR: Failed to allocate state structure\n");
gdbg("%s state structure \n", clog_message_str[CMN_LOG_ALLOC_FAIL]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for state structure

@NayanaNagaraj23
os : Common log messages
f4fb92d 
Make changes in the debug log messages with the common messages
ritesh55555
ritesh55555 reviewed Oct 21, 2024
View reviewed changes
os/include/tinyara/common_logs/common_logs.h
/* Memory Operations */
"Memory allocation failure", //CMN_LOG_ALLOC_FAIL,
"Null pointer", //CMN_LOG_NULL_CHECK_FAIL,
"Potential memory leak", //CMN_LOG_MEM_LEAK,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments can be indented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neel-samsung neel-samsung neel-samsung left review comments

@gSahitya-samsung gSahitya-samsung gSahitya-samsung left review comments

@r-prabu r-prabu r-prabu requested changes

@Tirtha-Samsung Tirtha-Samsung Tirtha-Samsung left review comments

@ritesh55555 ritesh55555 ritesh55555 left review comments

@kishore-sn kishore-sn kishore-sn left review comments

@sunghan-chang sunghan-chang sunghan-chang left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@NayanaNagaraj23 @neel-samsung @sunghan-chang @kishore-sn @ritesh55555 @Tirtha-Samsung @r-prabu @gSahitya-samsung