View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000062OpenFOAMBugpublic2010-10-23 23:182011-07-28 13:05
Reporterwyldckat Assigned Touser2 
PrioritynoneSeverityfeatureReproducibilityN/A
Status resolvedResolutionfixed 
Summary0000062: Request to add "isPathAbsolute" method to the fileName class
DescriptionThis is more of a feature request then a bug report, but I believe it will reduce the probability of future bugs related to file/folder paths in new libraries to come.

I've already posted about this here: http://www.cfd-online.com/Forums/openfoam-bugs/78643-ska-c-reads-filename-read-dynamicmeshdict-without-expanding.html#post270003
If such a method had already existed in fileName, that bug report thread would possibly never have existed. Additionally, this aids future portability issues that may be left undetected (actually, it has already been detected unofficially and is indirectly referred to here: http://openfoamwiki.net/index.php/Tip_Cross_Compiling_OpenFOAM_1.7_in_Linux_For_Windows_with_MinGW#Resolution_of_paths_is_now_fixed ).
Additional InformationIf this has little or no priority to the development of OpenFOAM, I'm willing to create a patch for this and post it here, or send it via email.
TagsNo tags attached.

Activities

user2

2010-10-26 09:24

  ~0000092

Feel free to post your patch and I'll take a look

wyldckat

2010-10-31 15:09

updater  

0001-Added-isPathAbsolute-to-fileName-and-made-changes-re.patch.gz (1,663 bytes)

wyldckat

2010-10-31 15:16

updater   ~0000108

Patch uploaded.
NOTE: I haven't made any changes to "src/dynamicFvMesh/solidBodyMotionFvMesh/solidBodyMotionFunctions/SKA/SKA.C", but I think that the treatment given in "src/thermophysicalModels/reactionThermo/chemistryReaders/chemkinReader/chemkinReader.C" to validate absolute or relative path for "CHEMKINFile" and "CHEMKINThermoFile", should also be given to "timeDataFileName" in SKA.C.

user2

2010-11-02 17:11

  ~0000113

Thanks for the patch - we'll look to include the functionality into the next release

wyldckat

2011-06-18 23:25

updater   ~0000448

Greetings Andy,

Now that the next release of OpenFOAM has been made, I've verified the application of the patch I submitted in this report and noticed that the file "src/thermophysicalModels/reactionThermo/chemistryReaders/chemkinReader/chemkinReader.C" was not updated to use the new method "isAbsolute()". It's the very last patch in the attached file.


Actually, I've re-checked how the method was defined and I believe that it's simpler than the attached patch for this specific file:
- Instead of "if (chemkinFile.size() && chemkinFile[0] != '/')"
- Simply use "if (!chemkinFile.isAbsolute())"
- Ditto for "thermoFile" a couple of lines below.

Best regards,
Bruno

user2

2011-06-20 10:48

  ~0000462

Indeed - this one was missed

Change made by commit 9a6fa75d588ba6ed008f1530c06255f5e93bda69

wyldckat

2011-06-28 17:06

updater   ~0000496

Just one last optimization, which is attached as "last_patch4includeEntry.gz".
It's for "src/OpenFOAM/db/dictionary/functionEntries/includeEntry/includeEntry.C", using the same optimization from my previous comment.

wyldckat

2011-06-28 17:06

updater  

last_patch4includeEntry.gz (288 bytes)

wyldckat

2011-07-21 22:27

updater  

last_patch4includeEntry (613 bytes)    
diff --git a/src/OpenFOAM/db/dictionary/functionEntries/includeEntry/includeEntry.C b/src/OpenFOAM/db/dictionary/functionEntries/includeEntry/includeEntry.C
index c45ef4f..dd69661 100644
--- a/src/OpenFOAM/db/dictionary/functionEntries/includeEntry/includeEntry.C
+++ b/src/OpenFOAM/db/dictionary/functionEntries/includeEntry/includeEntry.C
@@ -75,7 +75,7 @@ Foam::fileName Foam::functionEntries::includeEntry::includeFileName
     fName.expand();
 
     // relative name
-    if (fName.size() && !fName.isAbsolute())
+    if (!fName.isAbsolute())
     {
         fName = fileName(is.name()).path()/fName;
     }
last_patch4includeEntry (613 bytes)   

wyldckat

2011-07-21 22:31

updater   ~0000583

I'm sorry for uploading the last patch as a gzip file, when it's so small that it would be faster to see the text file directly, instead of having to download and check its contents.

I've re-uploaded the unpacked file "last_patch4includeEntry". Basically the method "isAbsolute" already checks if (in this case) "chemkinFile" is not empty, so it's not necessary to check it's size first.

user2

2011-07-28 13:05

  ~0000596

Update applied in commit 0f31c73

Issue History

Date Modified Username Field Change
2010-10-23 23:18 wyldckat New Issue
2010-10-26 09:24 user2 Note Added: 0000092
2010-10-26 09:24 user2 Assigned To => user2
2010-10-26 09:24 user2 Status new => acknowledged
2010-10-31 15:09 wyldckat File Added: 0001-Added-isPathAbsolute-to-fileName-and-made-changes-re.patch.gz
2010-10-31 15:16 wyldckat Note Added: 0000108
2010-11-02 17:11 user2 Note Added: 0000113
2010-11-02 17:11 user2 Status acknowledged => closed
2010-11-02 17:11 user2 Resolution open => no change required
2011-06-18 23:25 wyldckat Note Added: 0000448
2011-06-18 23:25 wyldckat Status closed => feedback
2011-06-18 23:25 wyldckat Resolution no change required => reopened
2011-06-20 10:48 user2 Note Added: 0000462
2011-06-20 10:48 user2 Status feedback => resolved
2011-06-20 10:48 user2 Fixed in Version => 2.0.x
2011-06-20 10:48 user2 Resolution reopened => fixed
2011-06-28 17:06 wyldckat Note Added: 0000496
2011-06-28 17:06 wyldckat Status resolved => feedback
2011-06-28 17:06 wyldckat Resolution fixed => reopened
2011-06-28 17:06 wyldckat File Added: last_patch4includeEntry.gz
2011-07-21 22:27 wyldckat File Added: last_patch4includeEntry
2011-07-21 22:31 wyldckat Note Added: 0000583
2011-07-21 22:31 wyldckat Status feedback => assigned
2011-07-28 13:05 user2 Note Added: 0000596
2011-07-28 13:05 user2 Status assigned => resolved
2011-07-28 13:05 user2 Resolution reopened => fixed