| Message ID | 1498219556-25179-1-git-send-email-mayhs11saini@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Fri, Jun 23, 2017 at 05:35:55PM +0530, Shyam Saini wrote: > Before the [Test] commit if we run nft with incomplete "add set" > command it caused segmentation fault and exit with error code 139 and > further it didn't throw any error message. > > For example: > $ sudo nft add set t s > > But after the [Test] commit it throws syntax error message and exits with > return value 1. > > For example: > $ sudo nft add set t s > <cmdline>:1:12-12: Error: syntax error, unexpected newline, expecting '{' > add set t s > ^ > > This commit tests changes made in the [Test] commit. Applied, thanks. I have reworked a bit your commit message, it looks a bit convoluted. No worries, have a look at what I pushed out for reference. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 26, 2017 at 10:29 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Fri, Jun 23, 2017 at 05:35:55PM +0530, Shyam Saini wrote: >> Before the [Test] commit if we run nft with incomplete "add set" >> command it caused segmentation fault and exit with error code 139 and >> further it didn't throw any error message. >> >> For example: >> $ sudo nft add set t s >> >> But after the [Test] commit it throws syntax error message and exits with >> return value 1. >> >> For example: >> $ sudo nft add set t s >> <cmdline>:1:12-12: Error: syntax error, unexpected newline, expecting '{' >> add set t s >> ^ >> >> This commit tests changes made in the [Test] commit. > > Applied, thanks. > > I have reworked a bit your commit message, it looks a bit convoluted. Thanks a lot :) > No worries, have a look at what I pushed out for reference. Shouldn't we follow conventions mentioned in "scripts/checkpatch.pl" ? Thanks, shyam -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 26, 2017 at 11:02:34PM +0530, Shyam Saini wrote: > On Mon, Jun 26, 2017 at 10:29 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Fri, Jun 23, 2017 at 05:35:55PM +0530, Shyam Saini wrote: > >> Before the [Test] commit if we run nft with incomplete "add set" > >> command it caused segmentation fault and exit with error code 139 and > >> further it didn't throw any error message. > >> > >> For example: > >> $ sudo nft add set t s > >> > >> But after the [Test] commit it throws syntax error message and exits with > >> return value 1. > >> > >> For example: > >> $ sudo nft add set t s > >> <cmdline>:1:12-12: Error: syntax error, unexpected newline, expecting '{' > >> add set t s > >> ^ > >> > >> This commit tests changes made in the [Test] commit. > > > > Applied, thanks. > > > > I have reworked a bit your commit message, it looks a bit convoluted. > > Thanks a lot :) > > > No worries, have a look at what I pushed out for reference. > > Shouldn't we follow conventions mentioned in "scripts/checkpatch.pl" ? Interesting. So the [test] thing is something that checkpatch.pl suggests, right? I would like to know more about that new thing, do you have documentation about this? I just tend to like that commit message are human-readable. I understand this structure makes it easier for robots, more simple to parse. So don't take checkpatch too seriously, probably too much engineering is going on there ;-) -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 26, 2017 at 11:07 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Mon, Jun 26, 2017 at 11:02:34PM +0530, Shyam Saini wrote: >> On Mon, Jun 26, 2017 at 10:29 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: >> > On Fri, Jun 23, 2017 at 05:35:55PM +0530, Shyam Saini wrote: >> >> Before the [Test] commit if we run nft with incomplete "add set" >> >> command it caused segmentation fault and exit with error code 139 and >> >> further it didn't throw any error message. >> >> >> >> For example: >> >> $ sudo nft add set t s >> >> >> >> But after the [Test] commit it throws syntax error message and exits with >> >> return value 1. >> >> >> >> For example: >> >> $ sudo nft add set t s >> >> <cmdline>:1:12-12: Error: syntax error, unexpected newline, expecting '{' >> >> add set t s >> >> ^ >> >> >> >> This commit tests changes made in the [Test] commit. >> > >> > Applied, thanks. >> > >> > I have reworked a bit your commit message, it looks a bit convoluted. >> >> Thanks a lot :) >> >> > No worries, have a look at what I pushed out for reference. >> >> Shouldn't we follow conventions mentioned in "scripts/checkpatch.pl" ? > > Interesting. > > So the [test] thing is something that checkpatch.pl suggests, right? yes something like that. > I would like to know more about that new thing, do you have > documentation about this? No documentation but yeah it throws following error when convention is not followed. "ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit c6cd7c22548a ("src: fix crash when inputting an incomplete set add command")'" > I just tend to like that commit message are human-readable. I > understand this structure makes it easier for robots, more simple to > parse. > So don't take checkpatch too seriously, probably too much engineering > is going on there ;-) Sure, Thanks for the correction -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 26, 2017 at 11:24:23PM +0530, Shyam Saini wrote: > On Mon, Jun 26, 2017 at 11:07 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Mon, Jun 26, 2017 at 11:02:34PM +0530, Shyam Saini wrote: > >> On Mon, Jun 26, 2017 at 10:29 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > >> > On Fri, Jun 23, 2017 at 05:35:55PM +0530, Shyam Saini wrote: > >> >> Before the [Test] commit if we run nft with incomplete "add set" > >> >> command it caused segmentation fault and exit with error code 139 and > >> >> further it didn't throw any error message. > >> >> > >> >> For example: > >> >> $ sudo nft add set t s > >> >> > >> >> But after the [Test] commit it throws syntax error message and exits with > >> >> return value 1. > >> >> > >> >> For example: > >> >> $ sudo nft add set t s > >> >> <cmdline>:1:12-12: Error: syntax error, unexpected newline, expecting '{' > >> >> add set t s > >> >> ^ > >> >> > >> >> This commit tests changes made in the [Test] commit. > >> > > >> > Applied, thanks. > >> > > >> > I have reworked a bit your commit message, it looks a bit convoluted. > >> > >> Thanks a lot :) > >> > >> > No worries, have a look at what I pushed out for reference. > >> > >> Shouldn't we follow conventions mentioned in "scripts/checkpatch.pl" ? > > > > Interesting. > > > > So the [test] thing is something that checkpatch.pl suggests, right? > yes something like that. > > > I would like to know more about that new thing, do you have > > documentation about this? > > No documentation but yeah it throws following error when convention is > not followed. > > "ERROR: Please use git commit description style 'commit <12+ chars of > sha1> ("<title line>")' - ie: 'commit c6cd7c22548a ("src: fix crash > when inputting an incomplete set add command")'" Ah, that's different thing. so no need for the [Test] tag you are adding. This means that: commit c6cd7c22548a ("src: fix crash when inputting an incomplete set add command") is the right way to refer to commits. That's a valid thing indeed, we should stick to that. Let's stick to this in follow up patches, OK? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 26, 2017 at 11:38 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Mon, Jun 26, 2017 at 11:24:23PM +0530, Shyam Saini wrote: >> On Mon, Jun 26, 2017 at 11:07 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: >> > On Mon, Jun 26, 2017 at 11:02:34PM +0530, Shyam Saini wrote: >> >> On Mon, Jun 26, 2017 at 10:29 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: >> >> > On Fri, Jun 23, 2017 at 05:35:55PM +0530, Shyam Saini wrote: >> >> >> Before the [Test] commit if we run nft with incomplete "add set" >> >> >> command it caused segmentation fault and exit with error code 139 and >> >> >> further it didn't throw any error message. >> >> >> >> >> >> For example: >> >> >> $ sudo nft add set t s >> >> >> >> >> >> But after the [Test] commit it throws syntax error message and exits with >> >> >> return value 1. >> >> >> >> >> >> For example: >> >> >> $ sudo nft add set t s >> >> >> <cmdline>:1:12-12: Error: syntax error, unexpected newline, expecting '{' >> >> >> add set t s >> >> >> ^ >> >> >> >> >> >> This commit tests changes made in the [Test] commit. >> >> > >> >> > Applied, thanks. >> >> > >> >> > I have reworked a bit your commit message, it looks a bit convoluted. >> >> >> >> Thanks a lot :) >> >> >> >> > No worries, have a look at what I pushed out for reference. >> >> >> >> Shouldn't we follow conventions mentioned in "scripts/checkpatch.pl" ? >> > >> > Interesting. >> > >> > So the [test] thing is something that checkpatch.pl suggests, right? >> yes something like that. >> >> > I would like to know more about that new thing, do you have >> > documentation about this? >> >> No documentation but yeah it throws following error when convention is >> not followed. >> >> "ERROR: Please use git commit description style 'commit <12+ chars of >> sha1> ("<title line>")' - ie: 'commit c6cd7c22548a ("src: fix crash >> when inputting an incomplete set add command")'" > > Ah, that's different thing. > > so no need for the [Test] tag you are adding. Sure > This means that: > > commit c6cd7c22548a ("src: fix crash when inputting an incomplete set add command") > > is the right way to refer to commits. That's a valid thing indeed, we > should stick to that. After adding "commit" keyword in the same commit message the error gone . Sorry, my bad. > Let's stick to this in follow up patches, OK? Sure Thanks a lot -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/shell/testcases/sets/0023incomplete_add_set_command_0 b/tests/shell/testcases/sets/0023incomplete_add_set_command_0 new file mode 100755 index 000000000000..b7535f7059db --- /dev/null +++ b/tests/shell/testcases/sets/0023incomplete_add_set_command_0 @@ -0,0 +1,16 @@ +#!/bin/bash + +# This testscase checks bug identified and fixed in the commit Id "c6cd7c22548a" +# Before the commit c6cd7c22548a, nft returns 139 (i.e, segmentation fault) which +# indicates the bug but after the commit it returns 1. + +$NFT add table t +$NFT add set t c + +ret=$? +if [ $ret -ne 1 ] ; +then + echo "E: returned $ret instead of 1" >&2 + exit 1 +fi +
Before the [Test] commit if we run nft with incomplete "add set" command it caused segmentation fault and exit with error code 139 and further it didn't throw any error message. For example: $ sudo nft add set t s But after the [Test] commit it throws syntax error message and exits with return value 1. For example: $ sudo nft add set t s <cmdline>:1:12-12: Error: syntax error, unexpected newline, expecting '{' add set t s ^ This commit tests changes made in the [Test] commit. Test:c6cd7c22548a ( "src: fix crash when inputting an incomplete set add command" ) Signed-off-by: Shyam Saini <mayhs11saini@gmail.com> --- .../testcases/sets/0023incomplete_add_set_command_0 | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100755 tests/shell/testcases/sets/0023incomplete_add_set_command_0