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.

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

Common Log (In Progress) #6338

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

Conversation

NayanaNagaraj23
Copy link
Contributor

Make changes in Debug log statements to reduce the binary size

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.

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
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?

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
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
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
os/pm/pm_metrics.c Outdated Show resolved Hide resolved
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.

"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.

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.


/* 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

"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.

@@ -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

@@ -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

@@ -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

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

Make changes in the debug log messages with the common messages
/* 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants