Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 43 additions & 15 deletions agent-system/src/SystemAgent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,23 +109,48 @@ remove_directory (const string& path, int depth)
*/
SystemAgent::SystemAgent ()
{
// #237481: problems with pids with too many digits: 64bits: max 20 digits
char tmp1[19+20];
snprintf (tmp1, sizeof(tmp1), "/tmp/YaST2-%05d-XXXXXX", getpid ());
}

/**
* @brief Temporary directory path
*
* Returns the temporary directory path, it creates the directory at the first
* call. The directory is automatically deleted in the destructor.
*
* @return string The returned path includes the current root (chroot) prefix.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return string The returned path includes the current root (chroot) prefix.
* @return string The returned path includes the current root (chroot) prefix.
For example "//tmp"

*/
string SystemAgent::tempdir ()
{
if (!_tempdir.empty()) {
return _tempdir;
}

const char* tmp2 = mkdtemp (tmp1);
// build the directory pattern: <scr_root>/<"tmp" if present>/YaST2-<pid>-XXXXXX
string dir_pattern(root());

// check if the "tmp" directory is present, if yes then use it
const char *tmp = "/tmp";
struct stat buf;
if (lstat((dir_pattern + tmp).c_str(), &buf) == 0 && S_ISDIR (buf.st_mode))
{
dir_pattern += tmp;
}

dir_pattern += "/YaST2-" + to_string(getpid()) + "-XXXXXX";

const char* tmp2 = mkdtemp ((char *)dir_pattern.c_str());
if (!tmp2)
{
std::ostringstream stm;
stm << "Cannot create temporary directory " << tmp1 << ':'
<< strerror (errno);
y2error ("%s", stm.str().c_str());
// #343258: terminate will print the uncaught exception too, unlike plain exit
throw std::runtime_error (stm.str());
string error = "Cannot create temporary directory " + dir_pattern + ':' + strerror(errno);
y2error ("%s", error.c_str());
// #343258: terminate will print the uncaught exception too, unlike plain exit
throw std::runtime_error (error);
}

tempdir = tmp2;
y2debug ("tmp directory is %s", tempdir.c_str ());
_tempdir = tmp2;
y2debug ("tmp directory is %s", _tempdir.c_str ());

return _tempdir;
}


Expand All @@ -135,7 +160,9 @@ SystemAgent::SystemAgent ()
SystemAgent::~SystemAgent ()
{
// remove temp directory and all its subdirectories.
remove_directory (tempdir.c_str(), 20);
if (!_tempdir.empty()) {
remove_directory (_tempdir.c_str(), 20);
}
}


Expand Down Expand Up @@ -332,7 +359,8 @@ SystemAgent::Read (const YCPPath& path, const YCPValue& arg, const YCPValue&)
* @example Read (.target.tmpdir) -> "/some/temp/dir"
*/

return YCPString (tempdir);
// skip the chroot prefix, return the location relative to the current root
return YCPString (tempdir().substr(strlen(root())));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is wrong :)

It is fine when tempdir is /mnt/tmp and root is /mnt
It is fine when tempdir is /tmp and root is empty
But what if root is /?

AFAIK SCRAgent::root is not clear about "" vs "/"

Yet your test shows it works... ... ah, because tempdir() will return two leading slashes. I think it deserves a comment in the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for safety I would also change code to something like (pseudocode) YCPString ((root() == "/" || root() == "") ? tempdir() : tempdir().substr(strlen(root())))

}

if (arg.isNull())
Expand Down Expand Up @@ -1022,7 +1050,7 @@ SystemAgent::Execute (const YCPPath& path, const YCPValue& value,
}
else if (cmd == "bash_output")
{
return shellcommand_output (root(), exports + bashcommand, tempdir);
return shellcommand_output (root(), exports + bashcommand, tempdir());
}
else if (cmd == "bash_background")
{
Expand Down
5 changes: 3 additions & 2 deletions agent-system/src/SystemAgent.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ class SystemAgent : public SCRAgent

private:

string tempdir;

// temporary directory, an absolute path with the current root (chroot) prefix!
string _tempdir;
string tempdir();
};


Expand Down
9 changes: 9 additions & 0 deletions package/yast2-core.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
-------------------------------------------------------------------
Mon Jul 25 14:19:37 UTC 2022 - Ladislav Slezák <[email protected]>

- Fixed ".target.tmpdir":
- Create the temporary directory in the target root (bsc#1199840)
- The temporary directory is now created only when needed
- Check if the target /tmp directory exists
- 4.5.3

-------------------------------------------------------------------
Mon Jul 4 13:46:48 UTC 2022 - Martin Liška <[email protected]>

Expand Down
2 changes: 1 addition & 1 deletion package/yast2-core.spec
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
%bcond_with werror

Name: yast2-core
Version: 4.5.2
Version: 4.5.3
Release: 0
Url: https://github.com/yast/yast-core

Expand Down