Warning fixes. #938

robertsipka · 2016-03-07T13:34:29Z

No description provided.

akosthekiss · 2016-03-07T13:48:18Z

jerry-core/mem/mem-poolman.c

 #ifndef MEM_HEAP_PTR_64
  uint32_t dummy; /* dummy member for alignment */
 #endif
-} mem_pools_chunk_t;
+} mem_pool_chunk;


This rewrite doesn't seem to be equivalent to the original version. This code defines a struct mem_pool_chunk -- and then also defines a variable named mem_pool_chunk of type struct mem_pool_chunk. This global variable is certainly not what we want.

BTW, why do we need this rewrite?

Yes, you are right. I changed this with another solution.
This modification fixed the following warnings:
150:22: assignment from incompatible pointer type : mem_free_chunk_p = chunk_p->next_p;
173:27: assignment from incompatible pointer type : chunk_to_free_p->next_p = mem_free_chunk_p;
190:39: initialization from incompatible pointer type : mem_pools_chunk_t *const next_p = mem_free_chunk_p->next_p;

zherczeg · 2016-03-07T14:34:57Z

jerry-core/parser/regexp/re-compiler.c

@@ -472,7 +472,7 @@ re_find_bytecode_in_cache (ecma_string_t *pattern_str_p, /**< pattern string */
          && ecma_compare_ecma_strings (cached_pattern_str_p, pattern_str_p))
      {
        JERRY_DDLOG ("RegExp is found in cache\n");
-        return re_cache[*idx];
+        return (re_compiled_code_t *) re_cache[*idx];


I would change re_cache to store re_compiled_code_t pointers. This is what is there anyway.

robertsipka · 2016-03-07T16:25:11Z

Thanks for the comments. I've updated the patch.
@zherczeg: At the first solution I didn't choose the safest way. I considered that and eliminated these warnings with adding the missing const modifiers, which I think is much closer to the original purpose. Please take a look at it.

akosthekiss · 2016-03-08T07:09:34Z

jerry-core/mem/mem-poolman.c

@@ -37,9 +37,9 @@
 /**
 * Node for free chunk list
 */
-typedef struct
+typedef struct mem_pools_chunk


Thanks for fixing this. However, there are some more recursive structs in the code base, which solve the the issue inconsitently, sometimes even incorrectly. Could you please open an issue or a follow-up PR to have a uniform solution for all recursive structs out there?

Agreed. I would prefer a naming like mem_pools_chunk_forward_t in general

LaszloLango · 2016-03-08T11:27:40Z

LGTM

zherczeg · 2016-03-08T13:00:21Z

LGTM

franc0is · 2016-03-09T01:18:13Z

jerry-core/parser/regexp/re-compiler.c

@@ -388,7 +388,7 @@ re_parse_alternative (re_compiler_ctx_t *re_ctx_p, /**< RegExp compiler context

        ECMA_TRY_CATCH (empty,
                        re_parse_char_class (re_ctx_p->parser_ctx_p,
-                                             re_append_char_class,
+                                             (re_char_class_callback) re_append_char_class,


In my opinion, it is bad practice to cast function pointers. Instead, change re_append_char_class to match re_char_class_callback's type and cast the uint32_t to ecma_char_t inside the function.

@franc0is, good point. @robertsipka, please change typedef void (*re_char_class_callback) (void *re_ctx_p, uint32_t start, uint32_t end); to typedef void (*re_char_class_callback) (void *re_ctx_p, ecma_char_t start, ecma_char_t end); and remove the cast.

Yes, thanks for the comments. I've updated the patch.
@LaszloLango : This modification will affect some helper functions, and more code refactoring needed. I think we could open a follow-up PR to do this kind of conversions.

Passing argument 1 of ‘strncmp’ from incompatible pointer type. Assignments from incompatible pointer types. Passing argument or initialization discards ‘const’ qualifier from pointer target type. JerryScript-DCO-1.0-Signed-off-by: Robert Sipka [email protected]

robertsipka · 2016-03-09T08:42:16Z

Rebased with master.

robertsipka force-pushed the warning_fixes branch from 0171aef to 50d3aa8 Compare March 7, 2016 13:37

akosthekiss reviewed Mar 7, 2016
View reviewed changes

robertsipka force-pushed the warning_fixes branch from 50d3aa8 to cfdeeb9 Compare March 7, 2016 14:19

zherczeg reviewed Mar 7, 2016
View reviewed changes

robertsipka force-pushed the warning_fixes branch from cfdeeb9 to 4b29727 Compare March 7, 2016 16:23

akosthekiss reviewed Mar 8, 2016
View reviewed changes

LaszloLango added the bug label Mar 8, 2016

franc0is reviewed Mar 9, 2016
View reviewed changes

robertsipka force-pushed the warning_fixes branch from 4b29727 to a9b5c31 Compare March 9, 2016 08:11

Warning fixes.

0b5a49f

Passing argument 1 of ‘strncmp’ from incompatible pointer type. Assignments from incompatible pointer types. Passing argument or initialization discards ‘const’ qualifier from pointer target type. JerryScript-DCO-1.0-Signed-off-by: Robert Sipka [email protected]

robertsipka force-pushed the warning_fixes branch from a9b5c31 to 0b5a49f Compare March 9, 2016 08:41

LaszloLango merged commit 0b5a49f into jerryscript-project:master Mar 9, 2016

LaszloLango mentioned this pull request Mar 18, 2016

Use code unit instead of code point #961

Merged

zherczeg mentioned this pull request Apr 7, 2016

Nominating Akos Kiss (akiss77) for JerryScript Maintainer status #992

Closed

zherczeg mentioned this pull request Sep 5, 2016

Nominating Robert Sipka (@robertsipka) to become a JerryScript committer #1321

Closed

robertsipka deleted the warning_fixes branch November 24, 2016 08:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warning fixes. #938

Warning fixes. #938

robertsipka commented Mar 7, 2016

akosthekiss Mar 7, 2016

robertsipka Mar 7, 2016

zherczeg Mar 7, 2016

robertsipka commented Mar 7, 2016

akosthekiss Mar 8, 2016

zherczeg Mar 8, 2016

LaszloLango commented Mar 8, 2016

zherczeg commented Mar 8, 2016

franc0is Mar 9, 2016

LaszloLango Mar 9, 2016

robertsipka Mar 9, 2016

LaszloLango Mar 9, 2016

robertsipka commented Mar 9, 2016

Warning fixes. #938

Warning fixes. #938

Conversation

robertsipka commented Mar 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertsipka commented Mar 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaszloLango commented Mar 8, 2016

zherczeg commented Mar 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertsipka commented Mar 9, 2016