Skip to content

Implement IO port API #1201

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

Merged
LaszloLango merged 1 commit into from
Jul 14, 2016
Merged

Implement IO port API #1201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion jerry-core/jerry-api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ typedef bool (*jerry_object_property_foreach_t) (const jerry_value_t property_na
*/
#ifdef JERRY_ENABLE_LOG
extern int jerry_debug_level;
extern FILE *jerry_log_file;
#endif /* JERRY_ENABLE_LOG */

/**
Expand Down
54 changes: 48 additions & 6 deletions jerry-core/jerry-port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ extern "C"
* @{
*/

/**
* Target port functions for console output
*/
int jerry_port_logmsg (FILE *stream, const char *format, ...);
int jerry_port_errormsg (const char *format, ...);

/*
* Termination Port API
*
Expand Down Expand Up @@ -71,6 +65,54 @@ typedef enum
*/
void jerry_port_fatal (jerry_fatal_code_t code);

/*
* I/O Port API
*/

/**
* Print a string to the console. The function should implement a printf-like
* interface, where the first argument specifies a format string on how to
* stringify the rest of the parameter list.
*
* This function is only called with strings coming from the executed ECMAScript
* wanting to print something as the result of its normal operation.
*
* It should be the port that decides what a "console" is.
*
* Example: a libc-based port may implement this with vprintf().
*/
void jerry_port_console (const char *format, ...);

/**
* Jerry log levels. The levels are in severity order
* where the most serious levels come first.
*/
typedef enum
{
JERRY_LOG_LEVEL_ERROR, /**< the engine will terminate after the message is printed */
JERRY_LOG_LEVEL_WARNING, /**< a request is aborted, but the engine continues its operation */
JERRY_LOG_LEVEL_DEBUG, /**< debug messages from the engine, low volume */
JERRY_LOG_LEVEL_TRACE /**< detailed info about engine internals, potentially high volume */
} jerry_log_level_t;

/**
* Display or log a debug/error message. The function should implement a printf-like
* interface, where the first argument specifies the log level
* and the second argument specifies a format string on how to stringify the rest
* of the parameter list.
*
* This function is only called with messages coming from the jerry engine as
* the result of some abnormal operation or describing its internal operations
* (e.g., data structure dumps or tracing info).
*
* It should be the port that decides whether error and debug messages are logged to
* the console, or saved to a database or to a file.
*
* Example: a libc-based port may implement this with vfprintf(stderr) or
* vfprintf(logfile), or both, depending on log level.
*/
void jerry_port_log (jerry_log_level_t level, const char *format, ...);

/*
* Date Port API
*/
Expand Down
5 changes: 0 additions & 5 deletions jerry-core/jerry.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,6 @@ static const char *wrong_args_msg_p = "wrong type of argument";
*/
int jerry_debug_level = 0;

/**
* File, used for logging
*/
FILE *jerry_log_file = NULL;

#endif /* JERRY_ENABLE_LOG */

/**
Expand Down
8 changes: 4 additions & 4 deletions jerry-core/jrt/jrt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ extern void __noreturn jerry_unimplemented (const char *, const char *, const ch
#define JERRY_LOG(lvl, ...) \
do \
{ \
if (lvl <= jerry_debug_level && jerry_log_file) \
if (lvl <= jerry_debug_level) \
Copy link
Member

@akosthekiss akosthekiss Jul 14, 2016

Choose a reason for hiding this comment

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

IMHO, this check should not happen here but in the port implementation (if that impl cares about it). Likewise, jerry_debug_level shouldn't even be defined in jerry.c. Everything JERRY_LOG should do is just pass things on to jerry_port_log implemented in the port for it to decide what to do with it (as lvl is passed on as well).

In some sense, it should work similarly to the "fatal" logic. As far ar jerry-core is concerned, there is only a single jerry_port_fatal function to be called. However, the default port implementation adds some extras to it, namely the "abort_on_fail" functionality. However, that's a port-specific thing. Therefore, the static bool abort_on_fail variable is defined in jerry-port-default-fatal.c, and accessor methods jerry_port_default_set_abort_on_fail and jerry_port_default_is_abort_on_fail in jerry-port-default.h.

I can imagine a similar scenario with the logs. There could be default port-specific accessors, e.g., jerry_port_default_set_log_level and jerry_port_default_get_log_level in jerry-port-default.h and their implementation (along with any variables needed for that logic, say debug_level or log_level) in jerry-port-default-io.c. And then the line (void) level; /* default port implementation ignores the log level */ will go away (actually, the presence of this line already signals the problem that log levels are not checked at the right place, IMHO).

Additonal notes:

  • JERRY_D[D[D]]LOG macros call JERRY_LOG with 1, 2, and 3 as level parameters. This does not seem right in the light of the new log level enum.
  • There are still some printfs in the code (e.g., jrt-fatals.c, js-parser.c, js-parser-util.c, ecma-builtin-global.c, jmem-poolman.c, jmem-heap.c, js-parser-statm.c), even if mostly macro-guarded. When will these be rewritten to call the _log and _console functions?

Copy link
Member

@akosthekiss akosthekiss Jul 14, 2016

Choose a reason for hiding this comment

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

(Sorry, missed the "Note: after this PR," part. Cancel my last Q.)

Copy link
Contributor Author

@LaszloLango LaszloLango Jul 14, 2016

Choose a reason for hiding this comment

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

I disagree with the first part. JERRY_D[D[D]]LOG macros are all on debug level. There where no multiple debug levels in the proposed (and accepted) IO port API proposal (see: #964).

I think moving the debug level to the port is might be a good idea. I'll try to do it in a followup work.

Copy link
Member

@akosthekiss akosthekiss Jul 14, 2016

Choose a reason for hiding this comment

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

#ifdef JERRY_ENABLE_LOG
#define JERRY_LOG(lvl, ...) \
  do \
  { \
    if (lvl <= jerry_debug_level) \
    { \
      jerry_port_log (JERRY_LOG_LEVEL_DEBUG, __VA_ARGS__); \
    } \
  } \
  while (0)

#define JERRY_DLOG(...) JERRY_LOG (1, __VA_ARGS__)
#define JERRY_DDLOG(...) JERRY_LOG (2, __VA_ARGS__)
#define JERRY_DDDLOG(...) JERRY_LOG (3, __VA_ARGS__)
#else /* !JERRY_ENABLE_LOG */
#define JERRY_DLOG(...) \
  do \
  { \
    if (false) \
    { \
      jerry_ref_unused_variables (0, __VA_ARGS__); \
    } \
  } while (0)
#define JERRY_DDLOG(...) JERRY_DLOG (__VA_ARGS__)
#define JERRY_DDDLOG(...) JERRY_DLOG (__VA_ARGS__)
#endif /* JERRY_ENABLE_LOG */

This is current status. JERRY_D[D[D]]LOG macros expand to JERRY_LOG (]0-3], ...) depending on JERRY_ENABLE_LOG settings. The first parameter should be the debug level you just mentioned, not integers.

Copy link
Contributor Author

@LaszloLango LaszloLango Jul 14, 2016

Choose a reason for hiding this comment

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

@akosthekiss, as I said. I'll do it in a follow up work. Don't worry. It's on my list to clean this up.

{ \
jerry_port_logmsg (jerry_log_file, __VA_ARGS__); \
jerry_port_log (JERRY_LOG_LEVEL_DEBUG, __VA_ARGS__); \
} \
} \
while (0)
Expand All @@ -109,8 +109,8 @@ extern void __noreturn jerry_unimplemented (const char *, const char *, const ch
#define JERRY_DDDLOG(...) JERRY_DLOG (__VA_ARGS__)
#endif /* JERRY_ENABLE_LOG */

#define JERRY_ERROR_MSG(...) jerry_port_errormsg (__VA_ARGS__)
#define JERRY_WARNING_MSG(...) JERRY_ERROR_MSG (__VA_ARGS__)
#define JERRY_ERROR_MSG(...) jerry_port_log (JERRY_LOG_LEVEL_ERROR, __VA_ARGS__)
#define JERRY_WARNING_MSG(...) jerry_port_log (JERRY_LOG_LEVEL_WARNING, __VA_ARGS__)

/**
* Mark for unreachable points and unimplemented cases
Expand Down
76 changes: 19 additions & 57 deletions main-unix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ read_file (const char *file_name,
FILE *file = fopen (file_name, "r");
if (file == NULL)
{
jerry_port_errormsg ("Error: failed to open file: %s\n", file_name);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: failed to open file: %s\n", file_name);
return NULL;
}

size_t bytes_read = fread (buffer, 1u, sizeof (buffer), file);
if (!bytes_read)
{
jerry_port_errormsg ("Error: failed to read file: %s\n", file_name);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: failed to read file: %s\n", file_name);
fclose (file);
return NULL;
}
Expand Down Expand Up @@ -85,7 +85,7 @@ assert_handler (const jerry_value_t func_obj_val __attribute__((unused)), /**< f
}
else
{
jerry_port_errormsg ("Script error: assertion failed\n");
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Script error: assertion failed\n");
exit (JERRY_STANDALONE_EXIT_CODE_FAIL);
}
} /* assert_handler */
Expand Down Expand Up @@ -114,7 +114,6 @@ print_help (char *name)
" --save-snapshot-for-eval FILE\n"
" --exec-snapshot FILE\n"
" --log-level [0-3]\n"
" --log-file FILE\n"
" --abort-on-fail\n"
"\n",
name);
Expand All @@ -126,8 +125,10 @@ main (int argc,
{
if (argc > JERRY_MAX_COMMAND_LINE_ARGS)
{
jerry_port_errormsg ("Error: too many command line arguments: %d (JERRY_MAX_COMMAND_LINE_ARGS=%d)\n",
argc, JERRY_MAX_COMMAND_LINE_ARGS);
jerry_port_log (JERRY_LOG_LEVEL_ERROR,
"Error: too many command line arguments: %d (JERRY_MAX_COMMAND_LINE_ARGS=%d)\n",
argc,
JERRY_MAX_COMMAND_LINE_ARGS);

return JERRY_STANDALONE_EXIT_CODE_FAIL;
}
Expand All @@ -154,9 +155,6 @@ main (int argc,

bool is_repl_mode = false;

#ifdef JERRY_ENABLE_LOG
const char *log_file_name = NULL;
#endif /* JERRY_ENABLE_LOG */
for (i = 1; i < argc; i++)
{
if (!strcmp ("-h", argv[i]) || !strcmp ("--help", argv[i]))
Expand Down Expand Up @@ -193,14 +191,14 @@ main (int argc,

if (save_snapshot_file_name_p != NULL)
{
jerry_port_errormsg ("Error: snapshot file name already specified\n");
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: snapshot file name already specified\n");
print_usage (argv[0]);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}

if (++i >= argc)
{
jerry_port_errormsg ("Error: no file specified for %s\n", argv[i - 1]);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: no file specified for %s\n", argv[i - 1]);
print_usage (argv[0]);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}
Expand All @@ -211,7 +209,7 @@ main (int argc,
{
if (++i >= argc)
{
jerry_port_errormsg ("Error: no file specified for %s\n", argv[i - 1]);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: no file specified for %s\n", argv[i - 1]);
print_usage (argv[0]);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}
Expand All @@ -223,35 +221,21 @@ main (int argc,
{
if (++i >= argc)
{
jerry_port_errormsg ("Error: no level specified for %s\n", argv[i - 1]);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: no level specified for %s\n", argv[i - 1]);
print_usage (argv[0]);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}

if (strlen (argv[i]) != 1 || argv[i][0] < '0' || argv[i][0] > '3')
{
jerry_port_errormsg ("Error: wrong format for %s\n", argv[i - 1]);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: wrong format for %s\n", argv[i - 1]);
print_usage (argv[0]);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}

#ifdef JERRY_ENABLE_LOG
flags |= JERRY_INIT_ENABLE_LOG;
jerry_debug_level = argv[i][0] - '0';
#endif /* JERRY_ENABLE_LOG */
}
else if (!strcmp ("--log-file", argv[i]))
{
if (++i >= argc)
{
jerry_port_errormsg ("Error: no file specified for %s\n", argv[i - 1]);
print_usage (argv[0]);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}

#ifdef JERRY_ENABLE_LOG
flags |= JERRY_INIT_ENABLE_LOG;
log_file_name = argv[i];
#endif /* JERRY_ENABLE_LOG */
}
else if (!strcmp ("--abort-on-fail", argv[i]))
Expand All @@ -260,7 +244,7 @@ main (int argc,
}
else if (!strncmp ("-", argv[i], 1))
{
jerry_port_errormsg ("Error: unrecognized option: %s\n", argv[i]);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Error: unrecognized option: %s\n", argv[i]);
print_usage (argv[0]);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}
Expand All @@ -274,13 +258,15 @@ main (int argc,
{
if (files_counter != 1)
{
jerry_port_errormsg ("Error: --save-snapshot argument works with exactly one script\n");
jerry_port_log (JERRY_LOG_LEVEL_ERROR,
"Error: --save-snapshot argument works with exactly one script\n");
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}

if (exec_snapshots_count != 0)
{
jerry_port_errormsg ("Error: --save-snapshot and --exec-snapshot options can't be passed simultaneously\n");
jerry_port_log (JERRY_LOG_LEVEL_ERROR,
"Error: --save-snapshot and --exec-snapshot options can't be passed simultaneously\n");
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}
}
Expand All @@ -291,22 +277,6 @@ main (int argc,
is_repl_mode = true;
}

#ifdef JERRY_ENABLE_LOG
if (log_file_name)
{
jerry_log_file = fopen (log_file_name, "w");
if (jerry_log_file == NULL)
{
jerry_port_errormsg ("Error: failed to open log file: %s\n", log_file_name);
return JERRY_STANDALONE_EXIT_CODE_FAIL;
}
}
else
{
jerry_log_file = stdout;
}
#endif /* JERRY_ENABLE_LOG */

jerry_init (flags);

jerry_value_t global_obj_val = jerry_get_global_object ();
Expand All @@ -321,7 +291,7 @@ main (int argc,

if (!is_assert_added)
{
jerry_port_errormsg ("Warning: failed to register 'assert' method.");
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Warning: failed to register 'assert' method.");
}

jerry_value_t ret_value = jerry_create_undefined ();
Expand Down Expand Up @@ -466,14 +436,6 @@ main (int argc,
jerry_release_value (print_function);
}

#ifdef JERRY_ENABLE_LOG
if (jerry_log_file && jerry_log_file != stdout)
{
fclose (jerry_log_file);
jerry_log_file = NULL;
}
#endif /* JERRY_ENABLE_LOG */

int ret_code = JERRY_STANDALONE_EXIT_CODE_OK;

if (jerry_value_has_error_flag (ret_value))
Expand All @@ -489,7 +451,7 @@ main (int argc,
assert (sz == err_str_size);
err_str_buf[err_str_size] = 0;

jerry_port_errormsg ("Script Error: unhandled exception: %s\n", err_str_buf);
jerry_port_log (JERRY_LOG_LEVEL_ERROR, "Script Error: unhandled exception: %s\n", err_str_buf);

jerry_release_value (err_str_val);

Expand Down
30 changes: 15 additions & 15 deletions targets/default/jerry-port-default-io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,30 @@
#include "jerry-port.h"

/**
* Provide log message to filestream implementation for the engine.
* Provide console message implementation for the engine.
*/
int jerry_port_logmsg (FILE *stream, /**< stream pointer */
const char *format, /**< format string */
...) /**< parameters */
void
jerry_port_console (const char *format, /**< format string */
...) /**< parameters */
{
va_list args;
int count;
va_start (args, format);
count = vfprintf (stream, format, args);
vfprintf (stdout, format, args);
va_end (args);
return count;
} /* jerry_port_logmsg */
} /* jerry_port_console */

/**
* Provide error message to console implementation for the engine.
* Provide log message implementation for the engine.
*/
int jerry_port_errormsg (const char *format, /**< format string */
...) /**< parameters */
void
jerry_port_log (jerry_log_level_t level, /**< log level */
const char *format, /**< format string */
...) /**< parameters */
{
(void) level; /* default port implementation ignores the log level */

va_list args;
int count;
va_start (args, format);
count = vfprintf (stderr, format, args);
vfprintf (stderr, format, args);
va_end (args);
return count;
} /* jerry_port_errormsg */
} /* jerry_port_log */
Loading