Message ID | 20240509200022.253089-14-edliaw@google.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | None | expand |
Hi Edward, On Thu, 9 May 2024 19:58:05 +0000 Edward Liaw <edliaw@google.com> wrote: > _GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent > redefinition warnings. > > Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX") I show Mathieu's comment on this[1]. I have no strong opinion on this, but if you conclude to remove or change this line, please apply same change to this patch. [1] https://lore.kernel.org/638a7831-493c-4917-9b22-5aa663e9ee84@efficios.com > Signed-off-by: Edward Liaw <edliaw@google.com> I also added trivial comments that coming from my personal and humble preferrence below. Other than the above and the below comments, Reviewed-by: SeongJae Park <sj@kernel.org> > --- > tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c | 3 --- > .../damon/debugfs_target_ids_read_before_terminate_race.c | 2 -- > 2 files changed, 5 deletions(-) > > diff --git a/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c b/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c > index 0cc2eef7d142..7a17a03d555c 100644 > --- a/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c > +++ b/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c > @@ -2,9 +2,6 @@ > /* > * Author: SeongJae Park <sj@kernel.org> > */ > - > -#define _GNU_SOURCE > - > #include <fcntl.h> I'd prefer having one empty line between the comment and includes. > #include <stdbool.h> > #include <stdint.h> > diff --git a/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c b/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c > index b06f52a8ce2d..4aeac55ac93e 100644 > --- a/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c > +++ b/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c > @@ -2,8 +2,6 @@ > /* > * Author: SeongJae Park <sj@kernel.org> > */ > -#define _GNU_SOURCE > - > #include <fcntl.h> Ditto. And I realize I also forgot adding one empty line before the above #define line. That's why I'm saying this is just a trivial comment :) > #include <stdbool.h> > #include <stdint.h> > -- > 2.45.0.118.g7fe29c98d7-goog Thanks, SJ
On Thu, May 9, 2024 at 1:31 PM SeongJae Park <sj@kernel.org> wrote: > > Hi Edward, > > On Thu, 9 May 2024 19:58:05 +0000 Edward Liaw <edliaw@google.com> wrote: > > > _GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent > > redefinition warnings. > > > > Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX") > > I show Mathieu's comment on this[1]. I have no strong opinion on this, but if > you conclude to remove or change this line, please apply same change to this > patch. Will do, thanks for reviewing. > > [1] https://lore.kernel.org/638a7831-493c-4917-9b22-5aa663e9ee84@efficios.com > > > Signed-off-by: Edward Liaw <edliaw@google.com> > > I also added trivial comments that coming from my personal and humble > preferrence below. Other than the above and the below comments, > > Reviewed-by: SeongJae Park <sj@kernel.org> > > > --- > > tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c | 3 --- > > .../damon/debugfs_target_ids_read_before_terminate_race.c | 2 -- > > 2 files changed, 5 deletions(-) > > > > diff --git a/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c b/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c > > index 0cc2eef7d142..7a17a03d555c 100644 > > --- a/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c > > +++ b/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c > > @@ -2,9 +2,6 @@ > > /* > > * Author: SeongJae Park <sj@kernel.org> > > */ > > - > > -#define _GNU_SOURCE > > - > > #include <fcntl.h> > > I'd prefer having one empty line between the comment and includes. > > > #include <stdbool.h> > > #include <stdint.h> > > diff --git a/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c b/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c > > index b06f52a8ce2d..4aeac55ac93e 100644 > > --- a/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c > > +++ b/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c > > @@ -2,8 +2,6 @@ > > /* > > * Author: SeongJae Park <sj@kernel.org> > > */ > > -#define _GNU_SOURCE > > - > > #include <fcntl.h> > > Ditto. > > And I realize I also forgot adding one empty line before the above #define > line. That's why I'm saying this is just a trivial comment :) No problem, I will add it back in. Thanks, Edward > > > #include <stdbool.h> > > #include <stdint.h> > > -- > > 2.45.0.118.g7fe29c98d7-goog > > > Thanks, > SJ
diff --git a/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c b/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c index 0cc2eef7d142..7a17a03d555c 100644 --- a/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c +++ b/tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c @@ -2,9 +2,6 @@ /* * Author: SeongJae Park <sj@kernel.org> */ - -#define _GNU_SOURCE - #include <fcntl.h> #include <stdbool.h> #include <stdint.h> diff --git a/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c b/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c index b06f52a8ce2d..4aeac55ac93e 100644 --- a/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c +++ b/tools/testing/selftests/damon/debugfs_target_ids_read_before_terminate_race.c @@ -2,8 +2,6 @@ /* * Author: SeongJae Park <sj@kernel.org> */ -#define _GNU_SOURCE - #include <fcntl.h> #include <stdbool.h> #include <stdint.h>
_GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent redefinition warnings. Fixes: 809216233555 ("selftests/harness: remove use of LINE_MAX") Signed-off-by: Edward Liaw <edliaw@google.com> --- tools/testing/selftests/damon/debugfs_target_ids_pid_leak.c | 3 --- .../damon/debugfs_target_ids_read_before_terminate_race.c | 2 -- 2 files changed, 5 deletions(-)