* Re: [devel] [git update] packages/katrin: heads/master
@ 2007-08-22 9:13 ` Slava Semushin
2007-08-22 9:41 ` Denis Klimov
0 siblings, 1 reply; 2+ messages in thread
From: Slava Semushin @ 2007-08-22 9:13 UTC (permalink / raw)
To: ALT Devel discussion list
2007/8/22, Denis Klimov <zver / altlinux.org>:
> Update of /people/zver/packages/katrin.git
[...]
> --- a/src/tc/katrin-tc.c
> +++ b/src/tc/katrin-tc.c
> @@ -59,7 +59,8 @@ if (argc == 2 && (strcmp(argv[1], "remove") == 0 || strcmp(argv[1], "reload") ==
> //open tmp script file
> char prog[25] = "";
> strlcpy(prog, "/tmp/katrin-tc.XXXXXX", sizeof prog);
> - mktemp(prog);
> + mkstemp(prog);
> +
Предлагаю избавиться от strlcpy() и сразу инициализировать массив значением:
- char prog[25] = "";
- strlcpy(prog, "/tmp/katrin-tc.XXXXXX", sizeof prog);
+ char prog[] = "/tmp/katrin-tc.XXXXXX";
Также:
- надо проверять возвращаемое от mkstemp() значение
- mkstemp() возвращает файловый дескриптор для временного файла,
поэтому надо не надо открывать файл с помощью fopen(). Вместо этого
можно преобразовать дескриптор для open() в дескриптор для fopen() с
помощью fdopen() и потом записывать данные файл с помощью простого
fprintf(). Ну или не преобразовывать и писать с помощью write().
> FILE *fh = fopen(prog,"w");
>
> fwrite("#!/bin/sh\n",strlen("#!/bin/sh\n"),1,fh);
> @@ -111,15 +112,18 @@ if (argc == 2 && (strcmp(argv[1], "remove") == 0 || strcmp(argv[1], "reload") ==
> ret = chmod(prog, S_IRUSR | S_IWUSR | S_IXUSR);
> if (ret == -1) {
> err("Can't do chmod(2) for temporary file!");
> + unlink(prog);
> return EXIT_FAILURE;
> }
>
> if (fork() == 0)
> execl (prog, prog, NULL);
> + unlink(prog);
[...]
А вот тут получается нехорошая ситуация:
- Процесс форкается и его ребёнок запускает скрипт на выполнение
- Родитель тут же удаляет скрипт, который его ребёнок исполняет
Надо:
- либо в родителе ждать когда ребёнок закончит выполнение скрипта
- либо добавить в конец скрипта строчку "rm -f $0", то скрипт после
выполнения сам себя удалит (но кажется это не самая лучшая идея,
потому что значение переменной $0 не всегда будет являеться полным
путём к скрипту)
- либо вообще не делать форк (действительно, зачем?) и выполнить
скрипт как-нибудь иначе чем exec() а потом его удалить.
Кстати, можно также попробовать избавиться от chmod(2), если выставить
верный umask(2) перед созданием временного файла, с помощью
mkstemp(3).
--
+ Slava Semushin | slava.semushin @ gmail.com
+ ALT Linux Team | php-coder @ altlinux.ru
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [devel] [git update] packages/katrin: heads/master
2007-08-22 9:13 ` [devel] [git update] packages/katrin: heads/master Slava Semushin
@ 2007-08-22 9:41 ` Denis Klimov
0 siblings, 0 replies; 2+ messages in thread
From: Denis Klimov @ 2007-08-22 9:41 UTC (permalink / raw)
To: ALT Devel discussion list
Этот кусок кода планируется вообще убрать и заменить все это с помощью функции system(3)
On Wed, 22 Aug 2007 16:13:02 +0700 Slava Semushin wrote:
> 2007/8/22, Denis Klimov <zver / altlinux.org>:
> > Update of /people/zver/packages/katrin.git
> [...]
> > --- a/src/tc/katrin-tc.c
> > +++ b/src/tc/katrin-tc.c
> > @@ -59,7 +59,8 @@ if (argc == 2 && (strcmp(argv[1], "remove") == 0 || strcmp(argv[1], "reload") ==
> > //open tmp script file
> > char prog[25] = "";
> > strlcpy(prog, "/tmp/katrin-tc.XXXXXX", sizeof prog);
> > - mktemp(prog);
> > + mkstemp(prog);
> > +
>
> Предлагаю избавиться от strlcpy() и сразу инициализировать массив значением:
>
> - char prog[25] = "";
> - strlcpy(prog, "/tmp/katrin-tc.XXXXXX", sizeof prog);
> + char prog[] = "/tmp/katrin-tc.XXXXXX";
>
> Также:
> - надо проверять возвращаемое от mkstemp() значение
> - mkstemp() возвращает файловый дескриптор для временного файла,
> поэтому надо не надо открывать файл с помощью fopen(). Вместо этого
> можно преобразовать дескриптор для open() в дескриптор для fopen() с
> помощью fdopen() и потом записывать данные файл с помощью простого
> fprintf(). Ну или не преобразовывать и писать с помощью write().
>
> > FILE *fh = fopen(prog,"w");
> >
> > fwrite("#!/bin/sh\n",strlen("#!/bin/sh\n"),1,fh);
> > @@ -111,15 +112,18 @@ if (argc == 2 && (strcmp(argv[1], "remove") == 0 || strcmp(argv[1], "reload") ==
> > ret = chmod(prog, S_IRUSR | S_IWUSR | S_IXUSR);
> > if (ret == -1) {
> > err("Can't do chmod(2) for temporary file!");
> > + unlink(prog);
> > return EXIT_FAILURE;
> > }
> >
> > if (fork() == 0)
> > execl (prog, prog, NULL);
> > + unlink(prog);
> [...]
>
> А вот тут получается нехорошая ситуация:
> - Процесс форкается и его ребёнок запускает скрипт на выполнение
> - Родитель тут же удаляет скрипт, который его ребёнок исполняет
>
> Надо:
> - либо в родителе ждать когда ребёнок закончит выполнение скрипта
> - либо добавить в конец скрипта строчку "rm -f $0", то скрипт после
> выполнения сам себя удалит (но кажется это не самая лучшая идея,
> потому что значение переменной $0 не всегда будет являеться полным
> путём к скрипту)
> - либо вообще не делать форк (действительно, зачем?) и выполнить
> скрипт как-нибудь иначе чем exec() а потом его удалить.
>
> Кстати, можно также попробовать избавиться от chmod(2), если выставить
> верный umask(2) перед созданием временного файла, с помощью
> mkstemp(3).
>
--
Denis Klimov
zver
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2007-08-22 9:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-22 9:13 ` [devel] [git update] packages/katrin: heads/master Slava Semushin
2007-08-22 9:41 ` Denis Klimov
ALT Linux Team development discussions
This inbox may be cloned and mirrored by anyone:
git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \
devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru
public-inbox-index devel
Example config snippet for mirrors.
Newsgroup available over NNTP:
nntp://lore.altlinux.org/org.altlinux.lists.devel
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git