HDFS-16667. Use malloc for buffer allocation in uriparser2 by GauthamBanasandra · Pull Request #4576 · apache/hadoop

GauthamBanasandra · 2022-07-18T05:33:39Z

Description of PR

Currently, a variable is used to specify the array size in uriparser2 -

hadoop/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/third_party/uriparser2/uriparser2/uriparser2.c

Lines 71 to 79 in 34e548c

    
           static int parse_int(const char *first, const char *after_last) { 
        
           	const int size = after_last - first; 
        
           	if (size) { 
        
           		char buffer[size + 1]; 
        
           		memcpyz(buffer, first, size); 
        
           		return atoi(buffer); 
        
           	} 
        
           	return 0; 
        
           }

This results in the following error on Windows -

H:\hadoop-hdfs-project\hadoop-hdfs-native-client\src\out\build\x64-Debug\cl : command line warning D9025: overriding '/W4' with '/w' 
    uriparser2.c
H:\hadoop-hdfs-project\hadoop-hdfs-native-client\src\main\native\libhdfspp\third_party\uriparser2\uriparser2\uriparser2.c(74,23): error C2057: expected constant expression 
H:\hadoop-hdfs-project\hadoop-hdfs-native-client\src\main\native\libhdfspp\third_party\uriparser2\uriparser2\uriparser2.c(74,23): error C2466: cannot allocate an array of constant size 0 
H:\hadoop-hdfs-project\hadoop-hdfs-native-client\src\main\native\libhdfspp\third_party\uriparser2\uriparser2\uriparser2.c(74,24): error C2133: 'buffer': unknown size

I've used malloc to fix this.

How was this patch tested?

Tested this locally by building on my Windows system.
Hadoop Jenkins CI validation.

For code changes:

Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

* Windows doesn't support variables for specifying the array size. * This PR uses malloc to fix this issue.

This reverts commit c5b46f1.

...-hdfs-native-client/src/main/native/libhdfspp/third_party/uriparser2/uriparser2/uriparser2.c

hadoop-yetus · 2022-07-19T09:08:18Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 51s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 1s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	85m 25s		trunk passed
+1 💚	compile	3m 58s		trunk passed
+1 💚	mvnsite	0m 38s		trunk passed
+1 💚	shadedclient	112m 10s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 20s		the patch passed
+1 💚	compile	3m 52s		the patch passed
+1 💚	cc	3m 52s		the patch passed
+1 💚	golang	3m 52s		the patch passed
+1 💚	javac	3m 52s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	mvnsite	0m 22s		the patch passed
+1 💚	shadedclient	22m 16s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	88m 30s		hadoop-hdfs-native-client in the patch passed.
+1 💚	asflicense	0m 51s		The patch does not generate ASF License warnings.
		231m 50s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/artifact/out/Dockerfile
GITHUB PR	#4576
Optional Tests	dupname asflicense compile cc mvnsite javac unit codespell detsecrets golang
uname	Linux c9777e842920 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `df2323c`
Default Java	Red Hat, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/testReport/
Max. process+thread count	615 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/console
versions	git=2.9.5 maven=3.6.3
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2022-07-19T12:55:34Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	29m 12s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	27m 29s		trunk passed
+1 💚	compile	4m 54s		trunk passed
+1 💚	mvnsite	0m 50s		trunk passed
+1 💚	shadedclient	58m 16s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 26s		the patch passed
+1 💚	compile	4m 12s		the patch passed
+1 💚	cc	4m 12s		the patch passed
+1 💚	golang	4m 12s		the patch passed
+1 💚	javac	4m 12s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	mvnsite	0m 27s		the patch passed
+1 💚	shadedclient	23m 42s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	107m 12s		hadoop-hdfs-native-client in the patch passed.
+1 💚	asflicense	0m 49s		The patch does not generate ASF License warnings.
		227m 4s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/artifact/out/Dockerfile
GITHUB PR	#4576
Optional Tests	dupname asflicense compile cc mvnsite javac unit codespell detsecrets golang
uname	Linux a10d05b9b421 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `df2323c`
Default Java	Red Hat, Inc.-1.8.0_312-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/testReport/
Max. process+thread count	623 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/console
versions	git=2.27.0 maven=3.6.3
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2022-07-19T17:10:47Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	27m 27s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	33m 54s		trunk passed
+1 💚	compile	4m 5s		trunk passed
+1 💚	mvnsite	0m 43s		trunk passed
+1 💚	shadedclient	71m 25s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 22s		the patch passed
+1 💚	compile	3m 56s		the patch passed
+1 💚	cc	3m 56s		the patch passed
+1 💚	golang	3m 56s		the patch passed
+1 💚	javac	3m 56s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	mvnsite	0m 27s		the patch passed
+1 💚	shadedclient	33m 43s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	113m 38s		hadoop-hdfs-native-client in the patch passed.
+1 💚	asflicense	0m 51s		The patch does not generate ASF License warnings.
		255m 0s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/artifact/out/Dockerfile
GITHUB PR	#4576
Optional Tests	dupname asflicense compile cc mvnsite javac unit codespell detsecrets golang
uname	Linux 7eb9a1960edb 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `df2323c`
Default Java	Debian-11.0.15+10-post-Debian-1deb10u1
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/testReport/
modules	C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/console
versions	git=2.20.1 maven=3.6.0
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2022-07-19T20:34:00Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 23s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	28m 20s		trunk passed
+1 💚	compile	5m 9s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	4m 57s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	mvnsite	0m 45s		trunk passed
+1 💚	shadedclient	64m 44s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 22s		the patch passed
+1 💚	compile	4m 41s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	cc	4m 41s		the patch passed
+1 💚	golang	4m 41s		the patch passed
+1 💚	javac	4m 41s		the patch passed
+1 💚	compile	4m 41s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	cc	4m 41s		the patch passed
+1 💚	golang	4m 41s		the patch passed
+1 💚	javac	4m 41s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	mvnsite	0m 24s		the patch passed
+1 💚	shadedclient	25m 32s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	97m 6s		hadoop-hdfs-native-client in the patch passed.
+1 💚	asflicense	0m 40s		The patch does not generate ASF License warnings.
		202m 58s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/artifact/out/Dockerfile
GITHUB PR	#4576
Optional Tests	dupname asflicense compile cc mvnsite javac unit codespell detsecrets golang
uname	Linux 232d09e8622b 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `df2323c`
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/testReport/
Max. process+thread count	606 (vs. ulimit of 5500)
modules	C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4576/3/console
versions	git=2.25.1 maven=3.6.3
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

* Windows doesn't support variables for specifying the array size. * This PR uses malloc to fix this issue.

GauthamBanasandra added 3 commits July 18, 2022 10:59

HDFS-16667. Use malloc for buffer allocation in uriparser2

25b1675

* Windows doesn't support variables for specifying the array size. * This PR uses malloc to fix this issue.

Run CI

c5b46f1

Revert "Run CI"

4f43c44

This reverts commit c5b46f1.

GauthamBanasandra self-assigned this Jul 18, 2022

GauthamBanasandra added the HDFS label Jul 18, 2022

This comment was marked as outdated.

Sign in to view

goiri reviewed Jul 18, 2022

View reviewed changes

...-hdfs-native-client/src/main/native/libhdfspp/third_party/uriparser2/uriparser2/uriparser2.c Outdated Show resolved Hide resolved

GauthamBanasandra added 2 commits July 19, 2022 10:31

Use spaces instead of tabs

25b1da9

Use proper indentation

df2323c

goiri approved these changes Jul 19, 2022

View reviewed changes

GauthamBanasandra marked this pull request as ready for review July 20, 2022 16:21

GauthamBanasandra merged commit d07256a into apache:trunk Jul 20, 2022

GauthamBanasandra deleted the use-malloc branch July 20, 2022 16:27

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022

HDFS-16667. Use malloc for buffer allocation in uriparser2 (apache#4576)

557aba2

* Windows doesn't support variables for specifying the array size. * This PR uses malloc to fix this issue.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

HDFS-16667. Use malloc for buffer allocation in uriparser2#4576

HDFS-16667. Use malloc for buffer allocation in uriparser2#4576
GauthamBanasandra merged 5 commits intoapache:trunkfrom
GauthamBanasandra:use-malloc

GauthamBanasandra commented Jul 18, 2022

Uh oh!

This comment was marked as outdated.

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	static int parse_int(const char first, const char after_last) {
	const int size = after_last - first;
	if (size) {
	char buffer[size + 1];
	memcpyz(buffer, first, size);
	return atoi(buffer);
	}
	return 0;
	}

Comments

Conversation

GauthamBanasandra commented Jul 18, 2022

Description of PR

How was this patch tested?

For code changes:

Uh oh!

This comment was marked as outdated.

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

hadoop-yetus commented Jul 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants