[PATCH - alsa-utils 1/1] arecord: Remove only regular files

[PATCH - alsa-utils 1/1] arecord: Remove only regular files

[Alexander Volkov]

arecord removes a file before writing into it. It's not
appropriate in some cases. For example, if you a pass
a symlink to a file, then the symlink will be removed
while the user expects to record into the symlink's target.
Another case is recording into the device file. Some
modems provide a tty device file as a voice device.
And it's not possible to write into it under root with
arecord, because it removes the device file.

So check the type of a file before writing into it and
remove only regular files.

Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>

diff --git a/aplay/aplay.c b/aplay/aplay.c
index 459f7dd..1b2cdfc 100644
--- a/aplay/aplay.c
+++ b/aplay/aplay.c
@@ -2929,6 +2929,7 @@ static void capture(char *orig_name)
 	char *name = orig_name;	/* current filename */
 	char namebuf[PATH_MAX+1];
 	off64_t count, rest;		/* number of bytes to capture */
+	struct stat statbuf;
 
 	/* get number of bytes to capture */
 	count = calc_count();
@@ -2973,7 +2974,10 @@ static void capture(char *orig_name)
 			}
 			
 			/* open a new file */
-			remove(name);
+			if (!lstat(name, &statbuf)) {
+				if (S_ISREG(statbuf.st_mode))
+					remove(name);
+			}
 			fd = safe_open(name);
 			if (fd < 0) {
 				perror(name);
-- 
2.4.0

Re: [PATCH - alsa-utils 1/1] arecord: Remove only regular files

[Arun Raghavan]

On 21 September 2015 at 18:35, Alexander Volkov <a.volkov@rusbitech.ru> wrote:
> arecord removes a file before writing into it. It's not
> appropriate in some cases. For example, if you a pass
> a symlink to a file, then the symlink will be removed
> while the user expects to record into the symlink's target.
> Another case is recording into the device file. Some
> modems provide a tty device file as a voice device.
> And it's not possible to write into it under root with
> arecord, because it removes the device file.
>
> So check the type of a file before writing into it and
> remove only regular files.
>
> Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>
>
> diff --git a/aplay/aplay.c b/aplay/aplay.c
> index 459f7dd..1b2cdfc 100644
> --- a/aplay/aplay.c
> +++ b/aplay/aplay.c
> @@ -2929,6 +2929,7 @@ static void capture(char *orig_name)
>         char *name = orig_name; /* current filename */
>         char namebuf[PATH_MAX+1];
>         off64_t count, rest;            /* number of bytes to capture */
> +       struct stat statbuf;
>
>         /* get number of bytes to capture */
>         count = calc_count();
> @@ -2973,7 +2974,10 @@ static void capture(char *orig_name)
>                         }
>
>                         /* open a new file */
> -                       remove(name);
> +                       if (!lstat(name, &statbuf)) {
> +                               if (S_ISREG(statbuf.st_mode))
> +                                       remove(name);
> +                       }
>                         fd = safe_open(name);
>                         if (fd < 0) {
>                                 perror(name);

Would it work to just open with O_TRUNC?

-- Arun

Re: [PATCH - alsa-utils 1/1] arecord: Remove only regular files

[Александр Волков]

22.09.2015 16:24, Arun Raghavan пишет:
> On 21 September 2015 at 18:35, Alexander Volkov <a.volkov@rusbitech.ru> wrote:
>> arecord removes a file before writing into it. It's not
>> appropriate in some cases. For example, if you a pass
>> a symlink to a file, then the symlink will be removed
>> while the user expects to record into the symlink's target.
>> Another case is recording into the device file. Some
>> modems provide a tty device file as a voice device.
>> And it's not possible to write into it under root with
>> arecord, because it removes the device file.
>>
>> So check the type of a file before writing into it and
>> remove only regular files.
>>
>> Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>
>>
>> diff --git a/aplay/aplay.c b/aplay/aplay.c
>> index 459f7dd..1b2cdfc 100644
>> --- a/aplay/aplay.c
>> +++ b/aplay/aplay.c
>> @@ -2929,6 +2929,7 @@ static void capture(char *orig_name)
>>          char *name = orig_name; /* current filename */
>>          char namebuf[PATH_MAX+1];
>>          off64_t count, rest;            /* number of bytes to capture */
>> +       struct stat statbuf;
>>
>>          /* get number of bytes to capture */
>>          count = calc_count();
>> @@ -2973,7 +2974,10 @@ static void capture(char *orig_name)
>>                          }
>>
>>                          /* open a new file */
>> -                       remove(name);
>> +                       if (!lstat(name, &statbuf)) {
>> +                               if (S_ISREG(statbuf.st_mode))
>> +                                       remove(name);
>> +                       }
>>                          fd = safe_open(name);
>>                          if (fd < 0) {
>>                                  perror(name);
> Would it work to just open with O_TRUNC?
>
> -- Arun
I guess that "remove" is used here to exclude mutual access to the file.
We can record to the created file while some program may read the 
removed file.
Using O_TRUNC will break this use case.

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH - alsa-utils 1/1] arecord: Remove only regular files

[Takashi Iwai]

On Tue, 22 Sep 2015 16:07:21 +0200,
Александр Волков wrote:
> 
> 22.09.2015 16:24, Arun Raghavan пишет:
> > On 21 September 2015 at 18:35, Alexander Volkov <a.volkov@rusbitech.ru> wrote:
> >> arecord removes a file before writing into it. It's not
> >> appropriate in some cases. For example, if you a pass
> >> a symlink to a file, then the symlink will be removed
> >> while the user expects to record into the symlink's target.
> >> Another case is recording into the device file. Some
> >> modems provide a tty device file as a voice device.
> >> And it's not possible to write into it under root with
> >> arecord, because it removes the device file.
> >>
> >> So check the type of a file before writing into it and
> >> remove only regular files.
> >>
> >> Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>
> >>
> >> diff --git a/aplay/aplay.c b/aplay/aplay.c
> >> index 459f7dd..1b2cdfc 100644
> >> --- a/aplay/aplay.c
> >> +++ b/aplay/aplay.c
> >> @@ -2929,6 +2929,7 @@ static void capture(char *orig_name)
> >>          char *name = orig_name; /* current filename */
> >>          char namebuf[PATH_MAX+1];
> >>          off64_t count, rest;            /* number of bytes to capture */
> >> +       struct stat statbuf;
> >>
> >>          /* get number of bytes to capture */
> >>          count = calc_count();
> >> @@ -2973,7 +2974,10 @@ static void capture(char *orig_name)
> >>                          }
> >>
> >>                          /* open a new file */
> >> -                       remove(name);
> >> +                       if (!lstat(name, &statbuf)) {
> >> +                               if (S_ISREG(statbuf.st_mode))
> >> +                                       remove(name);
> >> +                       }
> >>                          fd = safe_open(name);
> >>                          if (fd < 0) {
> >>                                  perror(name);
> > Would it work to just open with O_TRUNC?
> >
> > -- Arun
> I guess that "remove" is used here to exclude mutual access to the file.
> We can record to the created file while some program may read the 
> removed file.
> Using O_TRUNC will break this use case.

Yeah, it's a good argument.  I applied your patch as is now.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel