Re: [devel] [git update] packages/katrin: heads/master

Re: [devel] [git update] packages/katrin: heads/master

[Slava Semushin]

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

Re: [devel] [git update] packages/katrin: heads/master

[Denis Klimov]

Этот кусок кода планируется вообще убрать и заменить все это с помощью функции 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