cleanup compat/ sources using spatch
cover cleanup compat/ sources using spatch
Sources under compat/ were excluded from cleanup using spatch since some of them were taken from an upstream and we didn't want to modify them so that merging in future updates would be simpler. However, most of the files in compat/ were created by us so it should be safe to apply cleanup to those files.
Denton Liu (2):
mingw: apply array.cocci rule
compat/*.[ch]: remove extern from function declarations using spatch
compat/mingw.c | 4 ++-- compat/mingw.h | 6 +++--- compat/win32/pthread.h | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-)
Signature · 1 line
2.23.0.248.g3a9dd8fb08
PATCH 1/2 mingw: apply array.cocci rule
After running Coccinelle on all sources inside compat/ that were created by us[1], it was found that compat/mingw.c violated an array.cocci rule in two places and, thus, a patch was generated. Apply this patch so that all compat/ sources created by us follows all cocci rules.
[1]: Do not run Coccinelle on files that are taken from some upstream because in case we need to pull updates from them, we would like to have diverged as little as possible in order to make merging updates simpler.
The following sources were determined to have been taken from some upstream:
* compat/regex/
* compat/inet_ntop.c
* compat/inet_pton.c
* compat/nedmalloc/
* compat/obstack.{c,h}
* compat/poll/
--- compat/mingw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
compat/mingw.c +2 -2
@@ -1265,7 +1265,7 @@ static wchar_t *make_environment_block(char **deltaenv) }ALLOC_ARRAY (result, size); -memcpy (result, wenv, size *sizeof (*wenv)); +COPY_ARRAY (result, wenv, size);FreeEnvironmentStringsW (wenv);return result; }
@@ -1309,7 +1309,7 @@ static wchar_t *make_environment_block(char **deltaenv)continue ; size =wcslen (array[i]) +1 ; -memcpy (p, array[i], size *sizeof (*p)); +COPY_ARRAY (p, array[i], size); p += size; } *p =L'\0' ;
Signature · 1 line
2.23.0.248.g3a9dd8fb08
Re: [PATCH 1/2] mingw: apply array.cocci rule
Hi Denton,
Quoted 38 lines · Denton Liu
After running Coccinelle on all sources inside compat/ that were created by us[1], it was found that
compat/mingw.cviolated anarray.coccirule in two places and, thus, a patch was generated. Apply this patch so that all compat/ sources created by us follows all cocci rules.[1]: Do not run Coccinelle on files that are taken from some upstream because in case we need to pull updates from them, we would like to have diverged as little as possible in order to make merging updates simpler.
The following sources were determined to have been taken from some upstream:
* compat/regex/
*compat/inet_ntop.c
*compat/inet_pton.c
* compat/nedmalloc/
* compat/obstack.{c,h}
* compat/poll/Signed-off-by Denton Liu <[email protected]>--- compat/mingw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)compat/mingw.c +1
-1@@ -1265,7 +1265,7 @@ static wchar_t *make_environment_block(char **deltaenv) }ALLOC_ARRAY (result, size); -memcpy (result, wenv, size *sizeof (*wenv)); +COPY_ARRAY (result, wenv, size);FreeEnvironmentStringsW (wenv);return result; }
compat/mingw.c static wchar_t *make_environment_block(char **deltaenv) · line 1312
Quoted 5 lines
compat/mingw.c +1
-1@@ -1309,7 +1309,7 @@ static wchar_t *make_environment_block(char **deltaenv)continue ; size =wcslen (array[i]) +1 ; -memcpy (p, array[i], size *sizeof (*p)); +COPY_ARRAY (p, array[i], size);
ACK!
Thanks,
Dscho
Quoted 7 lines
compat/mingw.c +0
-1p += size; } *p =L'\0' ; --2.23.0.248.g3a9dd8fb08
Re: [PATCH 1/2] mingw: apply array.cocci rule
Quoted 44 lines · Johannes Schindelin
Hi Denton,
Quoted 40 lines · Denton Liu
After running Coccinelle on all sources inside compat/ that were created by us[1], it was found that
compat/mingw.cviolated anarray.coccirule in two places and, thus, a patch was generated. Apply this patch so that all compat/ sources created by us follows all cocci rules.[1]: Do not run Coccinelle on files that are taken from some upstream because in case we need to pull updates from them, we would like to have diverged as little as possible in order to make merging updates simpler.
The following sources were determined to have been taken from some upstream:
* compat/regex/
*compat/inet_ntop.c
*compat/inet_pton.c
* compat/nedmalloc/
* compat/obstack.{c,h}
* compat/poll/Signed-off-by Denton Liu <[email protected]>--- compat/mingw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)compat/mingw.c +1
-1@@ -1265,7 +1265,7 @@ static wchar_t *make_environment_block(char **deltaenv) }ALLOC_ARRAY (result, size); -memcpy (result, wenv, size *sizeof (*wenv)); +COPY_ARRAY (result, wenv, size);FreeEnvironmentStringsW (wenv);return result; }@@ -1309,7 +1309,7 @@ static wchar_t *make_environment_block(char **deltaenv)continue ;
Quoted 8 lines
compat/mingw.c static wchar_t *make_environment_block(char **deltaenv) · line 1312
Quoted 3 lines
compat/mingw.c +1
-1size =wcslen (array[i]) +1 ; -memcpy (p, array[i], size *sizeof (*p)); +COPY_ARRAY (p, array[i], size);ACK!
Thanks,
Dscho
Yup, looks good. Thanks both.
PATCH 2/2 compat/*.[ch]: remove extern from function declarations using spatch
In reply to Denton LiuIn 554544276a (*.[ch]: remove extern from function declarations using spatch, 2019-04-29), we removed externs from function declarations using spatch but we intentionally excluded files under compat/ since some are directly copied from an upstream and we should avoid churning them so that manually merging future updates will be simpler.
In the last commit, we determined the files which taken from an upstream so we can exclude them and run spatch on the remainder.
This was the Coccinelle patch used:
@@ type T; identifier f; @@ - extern T f(...);
and it was run with:
$ git ls-files compat/\*\*.{c,h} |
xargs spatch --sp-file contrib/coccinelle/noextern.cocci --in-place
$ git checkout -- \
compat/regex/ \
compat/inet_ntop.c \
compat/inet_pton.c \
compat/nedmalloc/ \
compat/obstack.{c,h} \
compat/poll/Coccinelle has some trouble dealing with __attribute__ and varargs so we ran the following to ensure that no remaining changes were left behind:
$ git ls-files compat/\*\*.{c,h} |
xargs sed -i'' -e 's/^\(\s*\)extern \([^(]*([^*]\)/\1\2/'
$ git checkout -- \
compat/regex/ \
compat/inet_ntop.c \
compat/inet_pton.c \
compat/nedmalloc/ \
compat/obstack.{c,h} \
compat/poll/--- compat/mingw.h | 6 +++--- compat/win32/pthread.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
compat/mingw.h +3 -3
@@ -11,7 +11,7 @@ typedef _sigset_t sigset_t;#undef _POSIX_THREAD_SAFE_FUNCTIONS#endif -extern int mingw_core_config (const char *var,const char *value,void *cb); +int mingw_core_config (const char *var,const char *value,void *cb);#define platform_core_config mingw_core_config/*
@@ -443,7 +443,7 @@ static inline void convert_slashes(char *path) *path ='/' ; }#define PATH_SEP ';' -extern char *mingw_query_user_email (void ); +char *mingw_query_user_email (void );#define query_user_email mingw_query_user_email#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER <1800 )#define PRIuMAX "I64u"
@@ -580,4 +580,4 @@ int main(int argc, const char **argv);/* * Used by Pthread API implementation for Windows */ -extern int err_win_to_posix (DWORD winerr); +int err_win_to_posix (DWORD winerr);
compat/win32/pthread.h +3 -3
@@ -50,7 +50,7 @@ typedef struct {DWORD tid; } pthread_t; -extern int pthread_create (pthread_t *thread,const void *unused, +int pthread_create (pthread_t *thread,const void *unused,void *(*start_routine)(void *),void *arg);/*
@@ -59,10 +59,10 @@ extern int pthread_create(pthread_t *thread, const void *unused,*/ #define pthread_join (a, b) win32_pthread_join(&(a), (b)) -extern int win32_pthread_join (pthread_t *thread,void **value_ptr); +int win32_pthread_join (pthread_t *thread,void **value_ptr);#define pthread_equal (t1, t2) ((t1).tid == (t2).tid) -extern pthread_t pthread_self (void ); +pthread_t pthread_self (void ); static inline void NORETURN pthread_exit(void *ret) {
Signature · 1 line
2.23.0.248.g3a9dd8fb08
Re: [PATCH 2/2] compat/*.[ch]: remove extern from function declarations using spatch
Hi Denton,
Quoted 34 lines · Denton Liu
In
554544276a(*.[ch]: remove extern from function declarations using spatch, 2019-04-29), we removed externs from function declarations using spatch but we intentionally excluded files under compat/ since some are directly copied from an upstream and we should avoid churning them so that manually merging future updates will be simpler.In the last commit, we determined the files which taken from an upstream so we can exclude them and run spatch on the remainder.
This was the Coccinelle patch used:
@@ type T; identifier f; @@ - extern T f(...);and it was run with:
$ git ls-files compat/\*\*.{c,h} | xargs spatch --sp-file contrib/coccinelle/noextern.cocci --in-place $ git checkout -- \ compat/regex/ \ compat/inet_ntop.c \ compat/inet_pton.c \ compat/nedmalloc/ \ compat/obstack.{c,h} \ compat/poll/Coccinelle has some trouble dealing with
__attribute__and varargs so we ran the following to ensure that no remaining changes were left behind:
Quoted 9 lines
$ git ls-files compat/\*\*.{c,h} |
xargs sed -i'' -e 's/^\(\s*\)extern \([^(]*([^*]\)/\1\2/'
$ git checkout -- \
compat/regex/ \
compat/inet_ntop.c \
compat/inet_pton.c \
compat/nedmalloc/ \
compat/obstack.{c,h} \
compat/poll/I wonder whether we want to make this part of the (slightly misnamed) "Static Analysis" job in our CI.
Quoted 49 lines
Signed-off-by Denton Liu <[email protected]>--- compat/mingw.h | 6 +++--- compat/win32/pthread.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)compat/mingw.h +3
-3@@ -11,7 +11,7 @@ typedef _sigset_t sigset_t;#undef _POSIX_THREAD_SAFE_FUNCTIONS#endif -extern int mingw_core_config (const char *var,const char *value,void *cb); +int mingw_core_config (const char *var,const char *value,void *cb);#define platform_core_config mingw_core_config/* @@ -443,7 +443,7 @@ static inline void convert_slashes(char *path) *path ='/' ; }#define PATH_SEP ';' -extern char *mingw_query_user_email (void ); +char *mingw_query_user_email (void );#define query_user_email mingw_query_user_email#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER <1800 )#define PRIuMAX "I64u"@@ -580,4 +580,4 @@ int main(int argc, const char **argv);/* * Used by Pthread API implementation for Windows */ -extern int err_win_to_posix (DWORD winerr); +int err_win_to_posix (DWORD winerr);compat/win32/pthread.h +1
-1@@ -50,7 +50,7 @@ typedef struct {DWORD tid; } pthread_t; -extern int pthread_create (pthread_t *thread,const void *unused, +int pthread_create (pthread_t *thread,const void *unused,void *(*start_routine)(void *),void *arg);/*
compat/win32/pthread.h extern int pthread_create(pthread_t *thread, const void *unused, · line 65
Quoted 9 lines
compat/win32/pthread.h +2
-2@@ -59,10 +59,10 @@ extern int pthread_create(pthread_t *thread, const void *unused,*/ #define pthread_join (a, b) win32_pthread_join(&(a), (b)) -extern int win32_pthread_join (pthread_t *thread,void **value_ptr); +int win32_pthread_join (pthread_t *thread,void **value_ptr);#define pthread_equal (t1, t2) ((t1).tid == (t2).tid) -extern pthread_t pthread_self (void ); +pthread_t pthread_self (void );
ACK!
Thanks,
Dscho
Quoted 7 lines
compat/win32/pthread.h +0
-1static inline void NORETURN pthread_exit(void *ret) { --2.23.0.248.g3a9dd8fb08
Re: [PATCH 2/2] compat/*.[ch]: remove extern from function declarations using spatch
Hi Dscho,
Quoted 48 lines · Johannes Schindelin
Hi Denton,
Quoted 34 lines · Denton Liu
In
554544276a(*.[ch]: remove extern from function declarations using spatch, 2019-04-29), we removed externs from function declarations using spatch but we intentionally excluded files under compat/ since some are directly copied from an upstream and we should avoid churning them so that manually merging future updates will be simpler.In the last commit, we determined the files which taken from an upstream so we can exclude them and run spatch on the remainder.
This was the Coccinelle patch used:
@@ type T; identifier f; @@ - extern T f(...);and it was run with:
$ git ls-files compat/\*\*.{c,h} | xargs spatch --sp-file contrib/coccinelle/noextern.cocci --in-place $ git checkout -- \ compat/regex/ \ compat/inet_ntop.c \ compat/inet_pton.c \ compat/nedmalloc/ \ compat/obstack.{c,h} \ compat/poll/Coccinelle has some trouble dealing with
__attribute__and varargs so we ran the following to ensure that no remaining changes were left behind:Quoted 9 lines
$ git ls-files compat/\*\*.{c,h} | xargs sed -i'' -e 's/^\(\s*\)extern \([^(]*([^*]\)/\1\2/' $ git checkout -- \ compat/regex/ \ compat/inet_ntop.c \ compat/inet_pton.c \ compat/nedmalloc/ \ compat/obstack.{c,h} \ compat/poll/
Quoted 2 lines
I wonder whether we want to make this part of the (slightly misnamed) "Static Analysis" job in our CI.
Do you mean running cocci on all of our source files as opposed to just the files we compile? These two patches are part of an experimental (and unsubmitted) patchset that does exactly that. Seeing that there's interest, I'll try to send it in soon.
Thanks,
Denton
Quoted 72 lines
Quoted 50 lines
Signed-off-by Denton Liu <[email protected]>--- compat/mingw.h | 6 +++--- compat/win32/pthread.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)compat/mingw.h +3
-3@@ -11,7 +11,7 @@ typedef _sigset_t sigset_t;#undef _POSIX_THREAD_SAFE_FUNCTIONS#endif -extern int mingw_core_config (const char *var,const char *value,void *cb); +int mingw_core_config (const char *var,const char *value,void *cb);#define platform_core_config mingw_core_config/* @@ -443,7 +443,7 @@ static inline void convert_slashes(char *path) *path ='/' ; }#define PATH_SEP ';' -extern char *mingw_query_user_email (void ); +char *mingw_query_user_email (void );#define query_user_email mingw_query_user_email#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER <1800 )#define PRIuMAX "I64u"@@ -580,4 +580,4 @@ int main(int argc, const char **argv);/* * Used by Pthread API implementation for Windows */ -extern int err_win_to_posix (DWORD winerr); +int err_win_to_posix (DWORD winerr);compat/win32/pthread.h +1
-1@@ -50,7 +50,7 @@ typedef struct {DWORD tid; } pthread_t; -extern int pthread_create (pthread_t *thread,const void *unused, +int pthread_create (pthread_t *thread,const void *unused,void *(*start_routine)(void *),void *arg);/* compat/win32/pthread.h extern int pthread_create(pthread_t *thread, const void *unused, · line 65
Quoted 9 lines
compat/win32/pthread.h +2
-2@@ -59,10 +59,10 @@ extern int pthread_create(pthread_t *thread, const void *unused,*/ #define pthread_join (a, b) win32_pthread_join(&(a), (b)) -extern int win32_pthread_join (pthread_t *thread,void **value_ptr); +int win32_pthread_join (pthread_t *thread,void **value_ptr);#define pthread_equal (t1, t2) ((t1).tid == (t2).tid) -extern pthread_t pthread_self (void ); +pthread_t pthread_self (void );ACK!
Thanks,
DschoQuoted 7 lines
compat/win32/pthread.h +0
-1static inline void NORETURN pthread_exit(void *ret) { --2.23.0.248.g3a9dd8fb08
Re: [PATCH 2/2] compat/*.[ch]: remove extern from function declarations using spatch
Hi Denton,
Quoted 48 lines · Denton Liu
Quoted 47 lines · Johannes Schindelin
Quoted 43 lines · Denton Liu
In
554544276a(*.[ch]: remove extern from function declarations using spatch, 2019-04-29), we removed externs from function declarations using spatch but we intentionally excluded files under compat/ since some are directly copied from an upstream and we should avoid churning them so that manually merging future updates will be simpler.In the last commit, we determined the files which taken from an upstream so we can exclude them and run spatch on the remainder.
This was the Coccinelle patch used:
@@ type T; identifier f; @@ - extern T f(...);and it was run with:
$ git ls-files compat/\*\*.{c,h} | xargs spatch --sp-file contrib/coccinelle/noextern.cocci --in-place $ git checkout -- \ compat/regex/ \ compat/inet_ntop.c \ compat/inet_pton.c \ compat/nedmalloc/ \ compat/obstack.{c,h} \ compat/poll/Coccinelle has some trouble dealing with
__attribute__and varargs so we ran the following to ensure that no remaining changes were left behind:$ git ls-files compat/\*\*.{c,h} | xargs sed -i'' -e 's/^\(\s*\)extern \([^(]*([^*]\)/\1\2/' $ git checkout -- \ compat/regex/ \ compat/inet_ntop.c \ compat/inet_pton.c \ compat/nedmalloc/ \ compat/obstack.{c,h} \ compat/poll/
Quoted 7 lines
Quoted 2 lines
I wonder whether we want to make this part of the (slightly misnamed) "Static Analysis" job in our CI.
Do you mean running cocci on all of our source files as opposed to just the files we compile? These two patches are part of an experimental (and unsubmitted) patchset that does exactly that. Seeing that there's interest, I'll try to send it in soon.
I look forward to it!
Dscho