| Message ID | 20240624015245.54968-2-lufei@uniontech.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | acct01: add EFAULT errno check. | expand |
Hi > Add EFAULT errno check in acct01 testcase. > Signed-off-by: lufei <lufei@uniontech.com> I guess you don't mind if I change this to following before merge: Signed-off-by: Lu Fei <lufei@uniontech.com> > --- > testcases/kernel/syscalls/acct/acct01.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > diff --git a/testcases/kernel/syscalls/acct/acct01.c b/testcases/kernel/syscalls/acct/acct01.c > index 1b53a32f2..ed1817bc5 100644 > --- a/testcases/kernel/syscalls/acct/acct01.c > +++ b/testcases/kernel/syscalls/acct/acct01.c > @@ -33,6 +33,7 @@ > #define FILE_TMPFILE "./tmpfile" > #define FILE_ELOOP "test_file_eloop1" > #define FILE_EROFS "ro_mntpoint/file" > +#define FILE_EFAULT "/tmp/invalid/file/name" And here I just amend to: #define FILE_EFAULT "invalid/file/name" (although it's very unlikely, the full patch *could* be existing, but not the relative one, because LTP is creating unique temporary directory for each run, e.g. /tmp/LTP_accTJpYqc. > static struct passwd *ltpuser; > @@ -45,6 +46,7 @@ static char *file_eloop; > static char *file_enametoolong; > static char *file_erofs; > static char *file_null; > +static char *file_efault; > static void setup_euid(void) > { > @@ -56,6 +58,16 @@ static void cleanup_euid(void) > SAFE_SETEUID(0); > } > +static void setup_emem(void) > +{ > + file_efault = SAFE_MMAP(NULL, 1, PROT_NONE, > + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); > +} > +static void cleanup_emem(void) > +{ > + SAFE_MUNMAP(file_efault, 1); > +} > + > static struct test_case { > char **filename; > char *desc; > @@ -72,6 +84,7 @@ static struct test_case { > {&file_eloop, FILE_ELOOP, ELOOP, NULL, NULL}, > {&file_enametoolong, "aaaa...", ENAMETOOLONG, NULL, NULL}, > {&file_erofs, FILE_EROFS, EROFS, NULL, NULL}, > + {&file_efault, FILE_EFAULT, EFAULT, setup_emem, cleanup_emem}, {&file_efault, "Invalid address", EFAULT, setup_emem, cleanup_emem}, (as Cyril suggested) Actually this second version does only single thing (unlike the previous version [1]), thus I suggest to merge it: Reviewed-by: Petr Vorel <pvorel@suse.cz> Therefore I reopen it in patchwork [2]. And you can send warning cleanup after merging this? Could you, please, next time put version to your patchset (e.g. -v2 for second version), this help to avoid confusions? Kind regards, Petr [1] https://lore.kernel.org/ltp/20240606065506.1686-1-lufei@uniontech.com/ [2] https://patchwork.ozlabs.org/project/ltp/patch/20240624015245.54968-2-lufei@uniontech.com/ > }; > static void setup(void)
Hi Petr, Cyril. Really thanks for the advises. Each of them makes me learned. Sorry for the patches not so good I've sent, I'm trying to make it better in the future. I will make warnings clearnup patches after this one merged. Best reguards. Lu Fei ------------------ Original ------------------ From: "Petr Vorel"<pvorel@suse.cz>; Date: Mon, Jul 8, 2024 04:23 AM To: "ltp"<ltp@lists.linux.it>; "lufei"<lufei@uniontech.com>; Cc: "Cyril Hrubis"<chrubis@suse.cz>; Subject: Re: [LTP] [PATCH] acct01: add EFAULT errno check. Hi > Add EFAULT errno check in acct01 testcase. > Signed-off-by: lufei <lufei@uniontech.com> I guess you don't mind if I change this to following before merge: Signed-off-by: Lu Fei <lufei@uniontech.com> > --- > testcases/kernel/syscalls/acct/acct01.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > diff --git a/testcases/kernel/syscalls/acct/acct01.c b/testcases/kernel/syscalls/acct/acct01.c > index 1b53a32f2..ed1817bc5 100644 > --- a/testcases/kernel/syscalls/acct/acct01.c > +++ b/testcases/kernel/syscalls/acct/acct01.c > @@ -33,6 +33,7 @@ > #define FILE_TMPFILE "./tmpfile" > #define FILE_ELOOP "test_file_eloop1" > #define FILE_EROFS "ro_mntpoint/file" > +#define FILE_EFAULT "/tmp/invalid/file/name" And here I just amend to: #define FILE_EFAULT "invalid/file/name" (although it's very unlikely, the full patch *could* be existing, but not the relative one, because LTP is creating unique temporary directory for each run, e.g. /tmp/LTP_accTJpYqc. > static struct passwd *ltpuser; > @@ -45,6 +46,7 @@ static char *file_eloop; > static char *file_enametoolong; > static char *file_erofs; > static char *file_null; > +static char *file_efault; > static void setup_euid(void) > { > @@ -56,6 +58,16 @@ static void cleanup_euid(void) > SAFE_SETEUID(0); > } > +static void setup_emem(void) > +{ > + file_efault = SAFE_MMAP(NULL, 1, PROT_NONE, > + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); > +} > +static void cleanup_emem(void) > +{ > + SAFE_MUNMAP(file_efault, 1); > +} > + > static struct test_case { > char **filename; > char *desc; > @@ -72,6 +84,7 @@ static struct test_case { > {&file_eloop, FILE_ELOOP, ELOOP, NULL, NULL}, > {&file_enametoolong, "aaaa...", ENAMETOOLONG, NULL, NULL}, > {&file_erofs, FILE_EROFS, EROFS, NULL, NULL}, > + {&file_efault, FILE_EFAULT, EFAULT, setup_emem, cleanup_emem}, {&file_efault, "Invalid address", EFAULT, setup_emem, cleanup_emem}, (as Cyril suggested) Actually this second version does only single thing (unlike the previous version [1]), thus I suggest to merge it: Reviewed-by: Petr Vorel <pvorel@suse.cz> Therefore I reopen it in patchwork [2]. And you can send warning cleanup after merging this? Could you, please, next time put version to your patchset (e.g. -v2 for second version), this help to avoid confusions? Kind regards, Petr [1] https://lore.kernel.org/ltp/20240606065506.1686-1-lufei@uniontech.com/ [2] https://patchwork.ozlabs.org/project/ltp/patch/20240624015245.54968-2-lufei@uniontech.com/ > }; > static void setup(void)
Hi Lu, > Hi Petr, Cyril. > Really thanks for the advises. Each of them makes me learned. You're welcome. Also looking at XHTML entity '>' and ' ' in your reply below, it would be great if you could set your email client (MUA) which you use for LTP mailing list to use plain text (not XHTML). Here are some tips: https://www.kernel.org/doc/html/latest/process/email-clients.html Kind regards, Petr > Sorry for the patches not so good I've sent, I'm trying to make it better in the future. > I will make warnings clearnup patches after this one merged. > Best reguards. > Lu Fei > > ------------------ Original ------------------ > From: "Petr Vorel"<pvorel@suse.cz>; > Date: Mon, Jul 8, 2024 04:23 AM > To: "ltp"<ltp@lists.linux.it>; "lufei"<lufei@uniontech.com>; > Cc: "Cyril Hrubis"<chrubis@suse.cz>; > Subject: Re: [LTP] [PATCH] acct01: add EFAULT errno check. > > Hi > > Add EFAULT errno check in acct01 testcase. > > Signed-off-by: lufei <lufei@uniontech.com> > I guess you don't mind if I change this to following before merge: > Signed-off-by: Lu Fei <lufei@uniontech.com> > > --- > > testcases/kernel/syscalls/acct/acct01.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > diff --git a/testcases/kernel/syscalls/acct/acct01.c b/testcases/kernel/syscalls/acct/acct01.c > > index 1b53a32f2..ed1817bc5 100644 > > --- a/testcases/kernel/syscalls/acct/acct01.c > > +++ b/testcases/kernel/syscalls/acct/acct01.c > > @@ -33,6 +33,7 @@ > > #define FILE_TMPFILE "./tmpfile" > > #define FILE_ELOOP "test_file_eloop1" > > #define FILE_EROFS "ro_mntpoint/file" > > +#define FILE_EFAULT "/tmp/invalid/file/name" ....
Hi Lu, merged, thanks! Kind regards, Petr
diff --git a/testcases/kernel/syscalls/acct/acct01.c b/testcases/kernel/syscalls/acct/acct01.c index 1b53a32f2..ed1817bc5 100644 --- a/testcases/kernel/syscalls/acct/acct01.c +++ b/testcases/kernel/syscalls/acct/acct01.c @@ -33,6 +33,7 @@ #define FILE_TMPFILE "./tmpfile" #define FILE_ELOOP "test_file_eloop1" #define FILE_EROFS "ro_mntpoint/file" +#define FILE_EFAULT "/tmp/invalid/file/name" static struct passwd *ltpuser; @@ -45,6 +46,7 @@ static char *file_eloop; static char *file_enametoolong; static char *file_erofs; static char *file_null; +static char *file_efault; static void setup_euid(void) { @@ -56,6 +58,16 @@ static void cleanup_euid(void) SAFE_SETEUID(0); } +static void setup_emem(void) +{ + file_efault = SAFE_MMAP(NULL, 1, PROT_NONE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); +} +static void cleanup_emem(void) +{ + SAFE_MUNMAP(file_efault, 1); +} + static struct test_case { char **filename; char *desc; @@ -72,6 +84,7 @@ static struct test_case { {&file_eloop, FILE_ELOOP, ELOOP, NULL, NULL}, {&file_enametoolong, "aaaa...", ENAMETOOLONG, NULL, NULL}, {&file_erofs, FILE_EROFS, EROFS, NULL, NULL}, + {&file_efault, FILE_EFAULT, EFAULT, setup_emem, cleanup_emem}, }; static void setup(void)
Add EFAULT errno check in acct01 testcase. Signed-off-by: lufei <lufei@uniontech.com> --- testcases/kernel/syscalls/acct/acct01.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)