Skip to content

Commit 94283d3

Browse files
author
Ahmed Bilal
committed
cmdLineUtils: fix rootmv same-file segfault and robust collection single-key handling
- Same-file move: rename in-place with kOverwrite; do not delete original. - Same-file copy/replace: clone first; delete only the clone. - Trees: CloneTree for copy; in-place rename for move. - Collections: detect via InheritsFrom('TCollection') and write with gDirectory.WriteObject for single-key. - Ruff: file is clean.
1 parent e4c6e10 commit 94283d3

File tree

1 file changed

+107
-43
lines changed

1 file changed

+107
-43
lines changed

main/python/cmdLineUtils.py

Lines changed: 107 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -649,81 +649,145 @@ def deleteObject(rootFile, pathSplit):
649649

650650
def copyRootObjectRecursive(sourceFile, sourcePathSplit, destFile, destPathSplit, replace, setName=""):
651651
"""
652-
Copy objects from a file or directory (sourceFile,sourcePathSplit)
653-
to an other file or directory (destFile,destPathSplit)
654-
- Has the will to be unix-like
655-
- that's a recursive function
656-
- Python adaptation of a root input/output tutorial : copyFiles.C
652+
Copy (or move) objects from (sourceFile,sourcePathSplit) to (destFile,destPathSplit).
653+
Handles rootcp and rootmv semantics. Special care for operations within the SAME file
654+
to avoid use-after-free when renaming or replacing.
657655
"""
658656
retcode = 0
659657
replaceOption = replace
660658
seen = {}
659+
660+
sameFile = sourceFile.GetName() == destFile.GetName()
661+
661662
for key in getKeyList(sourceFile, sourcePathSplit):
662663
objectName = key.GetName()
663664

664-
# write keys only if the cycle is higher than before
665-
if objectName not in seen.keys():
665+
# Keep only highest cycle for each name
666+
if objectName not in seen:
666667
seen[objectName] = key
667668
else:
668669
if seen[objectName].GetCycle() < key.GetCycle():
669670
seen[objectName] = key
670671
else:
671672
continue
672673

674+
# Directory case: recurse
673675
if isDirectoryKey(key):
674676
if not isExisting(destFile, destPathSplit + [objectName]):
675677
createDirectory(destFile, destPathSplit + [objectName])
676678
if isDirectory(destFile, destPathSplit + [objectName]):
677679
retcode += copyRootObjectRecursive(
678-
sourceFile, sourcePathSplit + [objectName], destFile, destPathSplit + [objectName], replace
680+
sourceFile, sourcePathSplit + [objectName],
681+
destFile, destPathSplit + [objectName],
682+
replaceOption
679683
)
680684
else:
681685
logging.warning(OVERWRITE_ERROR.format(objectName, objectName))
682686
retcode += 1
683-
elif isTreeKey(key):
684-
T = key.GetMotherDir().Get(objectName + ";" + str(key.GetCycle()))
685-
if replaceOption and isExisting(destFile, destPathSplit + [T.GetName()]):
686-
retcodeTemp = deleteObject(destFile, destPathSplit + [T.GetName()])
687+
continue
688+
689+
# Tree case
690+
if isTreeKey(key):
691+
T = key.GetMotherDir().Get(f"{objectName};{key.GetCycle()}")
692+
targetName = setName if setName else T.GetName()
693+
694+
# Same-file rename of tree (rootmv semantics)
695+
if sameFile and targetName != objectName and sourcePathSplit[:-1] == destPathSplit:
696+
# Handle potential existing destination name
697+
if isExisting(destFile, destPathSplit + [targetName]):
698+
if replaceOption:
699+
retcodeTemp = deleteObject(destFile, destPathSplit + [targetName])
700+
if retcodeTemp:
701+
retcode += retcodeTemp
702+
continue
703+
else:
704+
logging.warning(OVERWRITE_ERROR.format(targetName, targetName))
705+
retcode += 1
706+
continue
707+
changeDirectory(destFile, destPathSplit)
708+
T.SetName(targetName)
709+
# Overwrite ensures single cycle
710+
T.Write("", ROOT.TObject.kOverwrite)
711+
continue
712+
713+
# General copy/replace of tree
714+
if replaceOption and isExisting(destFile, destPathSplit + [targetName]):
715+
retcodeTemp = deleteObject(destFile, destPathSplit + [targetName])
687716
if retcodeTemp:
688717
retcode += retcodeTemp
689718
continue
690719
changeDirectory(destFile, destPathSplit)
691720
newT = T.CloneTree(-1, "fast")
692-
if setName != "":
693-
newT.SetName(setName)
721+
if targetName != newT.GetName():
722+
newT.SetName(targetName)
694723
newT.Write()
695-
else:
696-
obj = key.ReadObj()
697-
if replaceOption and isExisting(destFile, destPathSplit + [setName]):
698-
changeDirectory(destFile, destPathSplit)
699-
# Delete existing object before writing replacement
700-
retcodeTemp = deleteObject(destFile, destPathSplit + [setName])
701-
if retcodeTemp:
702-
retcode += retcodeTemp
703-
continue
704-
else:
705-
if isinstance(obj, ROOT.TNamed):
706-
obj.SetName(setName)
707-
changeDirectory(destFile, destPathSplit)
708-
obj.Write()
709-
elif issubclass(obj.__class__, ROOT.TCollection):
710-
# probably the object was written with kSingleKey
711-
changeDirectory(destFile, destPathSplit)
712-
obj.Write(setName, ROOT.TObject.kSingleKey)
713-
else:
714-
if replaceOption and isExisting(destFile, destPathSplit + [objectName]):
715-
retcodeTemp = deleteObject(destFile, destPathSplit + [objectName])
724+
# Delete only the clone, never original tree
725+
newT.Delete()
726+
continue
727+
728+
# Non-tree object
729+
obj = key.ReadObj()
730+
targetName = setName if setName else objectName
731+
732+
# Same-file rename (rootmv) where parent dirs are the same
733+
if sameFile and targetName != objectName and sourcePathSplit[:-1] == destPathSplit:
734+
# Destination exists?
735+
if isExisting(destFile, destPathSplit + [targetName]):
736+
if replaceOption:
737+
retcodeTemp = deleteObject(destFile, destPathSplit + [targetName])
716738
if retcodeTemp:
717739
retcode += retcodeTemp
718-
if setName != "":
719-
if isinstance(obj, ROOT.TNamed):
720-
obj.SetName(setName)
740+
obj.Delete()
741+
continue
721742
else:
722-
if isinstance(obj, ROOT.TNamed):
723-
obj.SetName(objectName)
724-
changeDirectory(destFile, destPathSplit)
725-
obj.Write()
726-
obj.Delete()
743+
logging.warning(OVERWRITE_ERROR.format(targetName, targetName))
744+
retcode += 1
745+
obj.Delete()
746+
continue
747+
# Perform in-place rename
748+
if isinstance(obj, ROOT.TNamed):
749+
obj.SetName(targetName)
750+
changeDirectory(destFile, destPathSplit)
751+
# Use kOverwrite so we do not create a new cycle
752+
obj.Write("", ROOT.TObject.kOverwrite)
753+
# IMPORTANT: Do NOT delete obj (it's original in same file)
754+
continue
755+
756+
# General same-file copy/replace: clone before deleting anything
757+
if sameFile:
758+
objToWrite = obj.Clone()
759+
else:
760+
objToWrite = obj
761+
762+
# Deletion step (only affects destination, do AFTER cloning)
763+
if replaceOption and targetName and isExisting(destFile, destPathSplit + [targetName]):
764+
retcodeTemp = deleteObject(destFile, destPathSplit + [targetName])
765+
if retcodeTemp:
766+
retcode += retcodeTemp
767+
# Clean up clone if created
768+
if objToWrite is not obj:
769+
objToWrite.Delete()
770+
continue
771+
772+
# Rename clone (or original if cross-file) if TNamed
773+
if isinstance(objToWrite, ROOT.TNamed) and targetName:
774+
objToWrite.SetName(targetName)
775+
776+
changeDirectory(destFile, destPathSplit)
777+
778+
if hasattr(objToWrite, 'InheritsFrom') and objToWrite.InheritsFrom('TCollection'):
779+
ROOT.gDirectory.WriteObject(objToWrite, targetName)
780+
else:
781+
objToWrite.Write()
782+
783+
# Delete only the temporary clone or cross-file object
784+
if objToWrite is not obj:
785+
objToWrite.Delete()
786+
else:
787+
# Cross-file original Python proxy corresponds to a newly read object; safe to delete
788+
if not sameFile:
789+
objToWrite.Delete()
790+
727791
changeDirectory(destFile, destPathSplit)
728792
ROOT.gDirectory.SaveSelf(ROOT.kTRUE)
729793
return retcode

0 commit comments

Comments
 (0)