You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "SahilKang (via GitHub)" <gi...@apache.org> on 2024/03/10 23:04:15 UTC

[PR] AVRO-3960: [C] Fix st ANYARGS warning [avro]

SahilKang opened a new pull request, #2798:
URL: https://github.com/apache/avro/pull/2798

   ## What is the purpose of the change
   
   This removes the following warning:
   
   ```
     avro/lang/c/src/st.c:240:13: warning: passing arguments to a function without a prototype is deprecated in all versions of C and is not
           supported in C2x [-Wdeprecated-non-prototype]
             hash_val = do_hash(key, table);
   ```
   
   ## Verifying this change
   
   This change is already covered by existing tests, such as `make test`
   
   ## Documentation
   
   No new features are added


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] AVRO-3960: [C] Fix st ANYARGS warning [avro]

Posted by "SahilKang (via GitHub)" <gi...@apache.org>.
SahilKang commented on PR #2798:
URL: https://github.com/apache/avro/pull/2798#issuecomment-1993353920

   good idea, I changed the macros to typedefs


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] AVRO-3960: [C] Fix st ANYARGS warning [avro]

Posted by "SahilKang (via GitHub)" <gi...@apache.org>.
SahilKang commented on code in PR #2798:
URL: https://github.com/apache/avro/pull/2798#discussion_r1522461364


##########
lang/c/src/st.c:
##########
@@ -212,7 +212,7 @@ void st_free_table(st_table *table)
 }
 
 #define PTR_NOT_EQUAL(table, ptr, hash_val, key) \
-((ptr) != 0 && (ptr->hash != (hash_val) || !EQUAL((table), (key), (ptr)->key)))
+((ptr) != 0 && (ptr->hash != (hash_val) || !EQUAL((table), (void*) (key), (void*) (ptr)->key)))

Review Comment:
   without the `void*` casts, we get `-Wpointer-integer-compare` warnings due to `st_data_t` being a `uintptr_t`
   
   the other call-sites throughout `/avro/lang/c` pass in pointers so the cast is only needed for `st.c`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] AVRO-3960: [C] Fix st ANYARGS warning [avro]

Posted by "mkmkme (via GitHub)" <gi...@apache.org>.
mkmkme commented on PR #2798:
URL: https://github.com/apache/avro/pull/2798#issuecomment-1990935219

   Thanks, it's good to have the deprecation warnings go away.
   
   One thing I would suggest, though, since `ANYARGS` is being replaced with three different function ptrs, is having those function ptr types as `typedef`-s and casting the functions in-place explicitly, without the macro.
   
   Does it sound reasonable?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] AVRO-3960: [C] Fix st ANYARGS warning [avro]

Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #2798:
URL: https://github.com/apache/avro/pull/2798


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] AVRO-3960: [C] Fix st ANYARGS warning [avro]

Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #2798:
URL: https://github.com/apache/avro/pull/2798#issuecomment-2017814184

   Thank you, @SahilKang !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] AVRO-3960: [C] Fix st ANYARGS warning [avro]

Posted by "mkmkme (via GitHub)" <gi...@apache.org>.
mkmkme commented on code in PR #2798:
URL: https://github.com/apache/avro/pull/2798#discussion_r1520964604


##########
lang/c/src/st.c:
##########
@@ -212,7 +212,7 @@ void st_free_table(st_table *table)
 }
 
 #define PTR_NOT_EQUAL(table, ptr, hash_val, key) \
-((ptr) != 0 && (ptr->hash != (hash_val) || !EQUAL((table), (key), (ptr)->key)))
+((ptr) != 0 && (ptr->hash != (hash_val) || !EQUAL((table), (void*) (key), (void*) (ptr)->key)))

Review Comment:
   Are the explicit casts to `void*` needed here and below for MSVC? Or is there some other reason to have them?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org