You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Rodrigo Schmidt (JIRA)" <ji...@apache.org> on 2010/06/01 12:44:36 UTC
[jira] Created: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
FileStatus allows null srcPath but crashes if that's done
---------------------------------------------------------
Key: HADOOP-6796
URL: https://issues.apache.org/jira/browse/HADOOP-6796
Project: Hadoop Common
Issue Type: Bug
Components: fs
Reporter: Rodrigo Schmidt
Assignee: Rodrigo Schmidt
Priority: Minor
FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Reopened: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tsz Wo (Nicholas), SZE reopened HADOOP-6796:
--------------------------------------------
Reopen since the patch has already been reverted.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874754#action_12874754 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
No problem. I'll submit a new patch soon.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Attachment: HADOOP-6796.patch
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Attachment: HADOOP-6796.1.patch
Does this new patch look better?
If so, I'll work on some unit tests.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877157#action_12877157 ]
Hudson commented on HADOOP-6796:
--------------------------------
Integrated in Hadoop-Common-trunk-Commit #291 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/291/])
HADOOP-6796. FileStatus allows null srcPath but crashes if that's done. Contributed by Rodrigo Schmidt.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874740#action_12874740 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
I think classes like FileStatus (eg writables) are special cases where the user has to know that they're using a constructor that leaves the object partially constructed. All the fields are invalid when using the default constructor, by the same logic we should also set a default owner, group etc instead of using null and converting to "". The path is not special (other parts of the system are not happy with "" users). While objects like this are less user friendly (they violate the rule that a constructor leaves an object in a valid state) they prevent you from allocating a bunch of objects that you're just going to overwrite. The default constructor for most of the other objects that implement writable (eg FsPermission) leave their fields as null as well. Its a trade off between performance and user friendlyness, the code currently sacrifies user friendlyness for performance. If we didn't do that you can see someone doing some profiling and following a jira that notices "we create a lot of objects in the default constructors that we immediately overwrite, let's avoid that". Not vetoing your patch, just pointing out that there's a reason the code works the way it does. This isn't a one off case where it's different, so we should be consistent.
Assertions are only enabled when the unit tests run, so there's no performance impact. I think asserts are good practice, not a maintenance burden, by that's my 2c.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Attachment: HADOOP-6796.3.patch
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Fix Version/s: 0.22.0
Affects Version/s: 0.22.0
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874361#action_12874361 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
If you can get rid of the FileStatus() constructor that's preferable since it's a useful invariant that objects are fully initialized after their constructors are called (users and methods don't need to worry about partially constructed objects) but that might not be possible or worth the trouble with FileStatus. I think it basically comes down to whether there is a good default value for a FileStatus path. Getting an NPE or hitting an assert when you try to use a partially constructed object seems preferable to accidentally using a value that wasn't intended (ie make users specify the path for a file status). My 2c.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874275#action_12874275 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
Shouldn't people who want a FileStatus that represents root just pass in "/"? It seems weird that new FileStatus() gives you a file status for root.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875486#action_12875486 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
Do you guys think I should add more checks to the method?
I could also check if the numbers are all non-negative, for example. path checking is the most important thing because it makes other methods fail.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Status: Patch Available (was: Open)
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874753#action_12874753 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
Seems reasonable to me. Thanks for all the back and forth!
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874347#action_12874347 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
So are you proposing to get rid of the FileStatus() constructor, and signal NPE when the user forgets to specify the srcPath?
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874396#action_12874396 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
You could also modify Path to accept Path("") or Path(null) so that instead of throwing an exception they silently convert to Path("/"). That runs into the same rationale, ie it's useful that Path throws an exception when given invalid arguments instead of converting the path to something else (this way people know when they pass in a null by mistake). You could also make sure FileStatus objects are created via a method that sets the required fields (ie use a factory).
What's the code that triggers this? If that code just wants a FileStatus that represents "/" then could you just modify that code to set the path explicitly to "/"?
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877878#action_12877878 ]
Tsz Wo (Nicholas), SZE commented on HADOOP-6796:
------------------------------------------------
The patch causes TestCopyFiles failing; see MAPREDUCE-1858. We probably should revert it because the constructor FileStatus() should not be changed.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874362#action_12874362 ]
Jakob Homan commented on HADOOP-6796:
-------------------------------------
bq. So are you proposing to get rid of the FileStatus() constructor, and signal NPE when the user forgets to specify the srcPath?
FileStatus is a Writable and therefore requires a default constructor, unfortunately. Were one to start looking at the classes that implement Writable, I imagine quite a few would be susceptible to this problem. It's a limitation of Writable (and Java...)
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Status: Open (was: Patch Available)
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878348#action_12878348 ]
Hudson commented on HADOOP-6796:
--------------------------------
Integrated in Hadoop-Common-trunk-Commit #302 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/302/])
HADOOP-6796. Reverting the patch.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874437#action_12874437 ]
Jakob Homan commented on HADOOP-6796:
-------------------------------------
bq. You could also modify Path to accept Path("") or Path(null) so that instead of throwing an exception they silently convert to Path("/")
Nope - silent conversions are evil.
The current patch is probably the best way to go, although this new behavior needs to be documented thoroughly. The general rule of thumb with anything implementing writable is caveat utilitor - let the user beware. Because of the write and readFields methods as well as the default constructor, it's always best practice to verify the state of the instance you're dealing with.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874338#action_12874338 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
In cases where you want to explicitly tolerate objects that are only partially constructed after calling the constructor (ie the user is responsible for initializing the internals) it's often useful to provide an isValid method that returns true if all the requisite fields have been initialized and public methods assert isValid before they're called. This way there's no side effects (ie you forget to set the src path but it happens to work because a default value was specified, but the user doesn't know what that is without looking at the object's internals) and the assert clearly indicates what field needs to be set. This feels like overkill to me. Objects that can be partially constructed are special cases where the user needs to know what fields are required, to me it's better to get an NPE indicating the user needs to set the source path than automatically constructing a path object for them using a value they may not be aware of.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874763#action_12874763 ]
Jakob Homan commented on HADOOP-6796:
-------------------------------------
Rodrigo: any constructor that converts an empty path to "/" is a change in behavior that we're not going to introduce. Again, from above and from what Eli said, Writable is fundamentally a flawed and dangerous data structure if you're expecting it to guarantee any integrity to its fields. Checking for not null on other constructors is fine, I guess; are we also going to verify that the constituent long fields are non-negative? And yeah, go ahead and throw in asserts - they're our best bet of catching logic errors in this data structure during unit tests.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Status: Open (was: Patch Available)
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877396#action_12877396 ]
Hudson commented on HADOOP-6796:
--------------------------------
Integrated in Hadoop-Common-trunk #362 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/362/])
HADOOP-6796. FileStatus allows null srcPath but crashes if that's done. Contributed by Rodrigo Schmidt.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878343#action_12878343 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
I'll revert the change. If we want to reintroduce we should restore fix users first but it's reasonable for existing users of writables to expect null to work, we'd need to modify them first, ie can't assume they're broken.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874212#action_12874212 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
Why does anyone create a FileStatus with a null path? Seems like we should make those callers all pass in "/" if that's the intended behavior rather than automatically convert null to Path("/") in the constructor. Adding a path != null assertion in the constructor should prevent future cases of people passing in null.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874001#action_12874001 ]
Hadoop QA commented on HADOOP-6796:
-----------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12446012/HADOOP-6796.patch
against trunk revision 949658.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 3 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/556/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/556/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/556/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/556/console
This message is automatically generated.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877127#action_12877127 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
+1
I'll commit soon.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874228#action_12874228 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
@Dmytro: srcPath is currently defined as a Path object and I don't want to change that. Calling 'new Path("")', crashes because "" is not a valid path. That's why I defaulted it to "/"
@Scott: No checks for getPath() == null afaik, on the other hand, if srcPath is null, all the method calls I mentioned will crash.
@Eli: You create a FileStatus with a null srcPath if you do 'stat = new FileStatus();'. This patch actually changes srcPath to Path("/") whenever it's null, which is exactly the behavior you proposed, but without triggering the exception when someone does that. I don't think passing null should be prevented, since we don't do that for other parameters like "owner" or "group". In such cases we default the value to empty strings. The equivalent for srcPath should be Path("/") IMO.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Dmytro Molkov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874175#action_12874175 ]
Dmytro Molkov commented on HADOOP-6796:
---------------------------------------
Did you think of using an empty string instead of root for path? like other strings (owner, group) do?
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Status: Patch Available (was: Open)
Updated to junit4
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874749#action_12874749 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
Okay! I see your point.
Maybe we should just prevent null srcPaths on constructors other than the default one. This at least makes it clear that users should not create FileStatus objects with null srcPaths.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875199#action_12875199 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
If it's never ok to pass null for the path (ie a bug in the code) then you can assert path != null or throw an AssertionError. If it's OK sometimes and not others than IllegalArgumentException seems appropriate, seems like it should never be legitimate to create a FileStatus w/o a path unless using the no argument constructor.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Status: Patch Available (was: Open)
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Scott Chen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874176#action_12874176 ]
Scott Chen commented on HADOOP-6796:
------------------------------------
Rodrigo: Will returning "/" change some of the existing behavior? Is there any code path that does the following?
{code}
if (getPath() == null) {
...
}
{code}
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt resolved HADOOP-6796.
-------------------------------------
Resolution: Won't Fix
It's probably better to leave things the way they are.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876587#action_12876587 ]
Hadoop QA commented on HADOOP-6796:
-----------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12446567/HADOOP-6796.2.patch
against trunk revision 952471.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 2 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/573/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/573/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/573/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/573/console
This message is automatically generated.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878344#action_12878344 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
Sure! Go ahead and revert it please.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874284#action_12874284 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
What should be the srcPath for "new FileStatus()" then?
I don't think disabling this constructor is a good choice. As a Writable, people might want to create a new empty object and read from some DataOutput (that was exactly how I found the problem, by the way :-)).
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875003#action_12875003 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
I just realized the FileStatus constructors don't throw any exception. What should we do if the parameters fail the sanity check?
Should I create a new exception? If I do so, this will have a strong impact on several parts of Hadoop.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874487#action_12874487 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
@Eli: I noticed this problem while writing a unit test for a different JIRA. The code didn't care much about the contents of the FileStatus object and that's why I was creating it with "new FileStatus()".
@Jakob: Besides the javadoc, where else should this be documented?
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rodrigo Schmidt updated HADOOP-6796:
------------------------------------
Attachment: HADOOP-6796.2.patch
Wrote some quick unit tests
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878381#action_12878381 ]
Hudson commented on HADOOP-6796:
--------------------------------
Integrated in Hadoop-Common-trunk #365 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/365/])
HADOOP-6796. Reverting the patch.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878345#action_12878345 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
Reverted the patch, confirmed TestCopyFiles passes.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12877045#action_12877045 ]
Hadoop QA commented on HADOOP-6796:
-----------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12446680/HADOOP-6796.3.patch
against trunk revision 952471.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 2 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/574/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/574/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/574/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/574/console
This message is automatically generated.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874382#action_12874382 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
Wow! I didn't know this would create so much discussion. I thought it was such a simple code cleanup and unit tests. :-)
@Eli: Unfortunately we can't get rid of the default constructor because FileStatus is a Writable(). So, I think your proposal boils down to just leaving it the way it is.
I don't like it because it leaves room for people to use the default constructor or a null srcPath for some corner case conditions and crash unexpectedly down the road with an NPE.
So, my opinion is that any valid Path is better than null as a default value, and "/" seems reasonable.
The options are the following:
1) Leave it the way it is
2) Use Path("/") if the srcPath is null (current patch)
3) Start accepting a null srcPath and change the methods that currently crash because of it
My patch does (2) because I didn't think it was worth implementing (3). However, I definitely prefer (3) to (1), so I could definitely implement it.
What do the others think?
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875472#action_12875472 ]
Jakob Homan commented on HADOOP-6796:
-------------------------------------
It's fine, I guess. The new patch won't cause any harm and may catch some error at some point. Since the stateSanityCheck() method is essentially one line that's called once, I'd just inline it.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874658#action_12874658 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
Sounds like the simple thing to do is have your test:
{code}
s = new FileStatus();
s.setPath("/something");
s.setXXX(...);
{code}
Or use FileStatus#read like the only other user of the default constructor (ThriftHadoopFileSystem). For the writable use case the reason the default values are null is that you know you're going to immediately overwrite them via read() and this way you don't pay the cost of instantiating an object you're just going to overwrite.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878188#action_12878188 ]
Jakob Homan commented on HADOOP-6796:
-------------------------------------
+1 on revert. Writables are dangerous, we'll just have to live with them.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12874719#action_12874719 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
I'm not worried about my test. I fixed it by using a complete FileStatus object.
I just don't like the idea of leaving the code the way it is. It looks buggy and next time someone bumps into the NPE error, he or she will create a JIRA and go through all this discussion again.
@Eli: Here are a couple of options that I thought about this morning. Let me know if they sound plausible to you:
1) Prevent srcPath from being null in any non-default constructor of FileStatus() by using asserts. For the default constructor, use Path("/").
or
2) If srcPath is null, prevent any calls to methods different from readFields() with asserts. (This will have some performance impact and will be hard to maintain).
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875518#action_12875518 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
I think just checking the path is fine.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Rodrigo Schmidt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12878203#action_12878203 ]
Rodrigo Schmidt commented on HADOOP-6796:
-----------------------------------------
I think it's just bad initialization. permission should be initialized with the constructor value.
Not sure if it's better to revert this or add the changes and better unit tests to cover these cases.
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eli Collins updated HADOOP-6796:
--------------------------------
Status: Resolved (was: Patch Available)
Hadoop Flags: [Reviewed]
Resolution: Fixed
I've committed this. Thanks Rodrigo!
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.2.patch, HADOOP-6796.3.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-6796) FileStatus allows null srcPath but
crashes if that's done
Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-6796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12875284#action_12875284 ]
Eli Collins commented on HADOOP-6796:
-------------------------------------
Looks reasonable to me. What do you think Jakob?
> FileStatus allows null srcPath but crashes if that's done
> ---------------------------------------------------------
>
> Key: HADOOP-6796
> URL: https://issues.apache.org/jira/browse/HADOOP-6796
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.22.0
> Reporter: Rodrigo Schmidt
> Assignee: Rodrigo Schmidt
> Priority: Minor
> Fix For: 0.22.0
>
> Attachments: HADOOP-6796.1.patch, HADOOP-6796.patch
>
>
> FileStatus allows constructor invocation with a null srcPath but many methods like write, readFields, compareTo, equals, and hashCode depend on this property.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.