diff mbox series

extrahd.cgi: Fix for Bug #12863

Message ID	20230602190116.1790314-1-jon.murphy@ipfire.org
State	Accepted
Commit	236e89ae87063bb1883875f76080d3f03c0351d5
Headers	From: Jon Murphy <jon.murphy@ipfire.org> To: development@lists.ipfire.org Subject: [PATCH] extrahd.cgi: Fix for Bug #12863 Date: Fri, 2 Jun 2023 14:01:16 -0500 Message-Id: <20230602190116.1790314-1-jon.murphy@ipfire.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: list Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org>
Series	extrahd.cgi: Fix for Bug #12863 \| extrahd.cgi: Fix for Bug #12863

Commit Message

jon June 2, 2023, 7:01 p.m. UTC

  -Fixes remove entries in 'extrahd' via the webinterface for extrahd.cgi file.

Suggested-by: Bernhard Bitsch <bbitsch@ipfire.org>
Signed-off-by: Jon Murphy <jon.murphy@ipfire.org>
---
 html/cgi-bin/extrahd.cgi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Tremer June 5, 2023, 10:42 a.m. UTC | #1

Reviewed-by: Michael Tremer <michael.tremer@ipfire.org>

The syntax pains me, but this is probably correct :)

> On 2 Jun 2023, at 20:01, Jon Murphy <jon.murphy@ipfire.org> wrote:
> 
> -Fixes remove entries in 'extrahd' via the webinterface for extrahd.cgi file.
> 
> Suggested-by: Bernhard Bitsch <bbitsch@ipfire.org>
> Signed-off-by: Jon Murphy <jon.murphy@ipfire.org>
> ---
> html/cgi-bin/extrahd.cgi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/html/cgi-bin/extrahd.cgi b/html/cgi-bin/extrahd.cgi
> index 36c0efc2e..9dd3a0eee 100644
> --- a/html/cgi-bin/extrahd.cgi
> +++ b/html/cgi-bin/extrahd.cgi
> @@ -103,7 +103,7 @@ END
> }
> elsif ($extrahdsettings{'ACTION'} eq $Lang::tr{'delete'})
> {
> - if ( &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
> + if ( ! &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
> open( FILE, "< $devicefile" ) or die "Unable to read $devicefile";
> @tmp = <FILE>;
> close FILE;
> -- 
> 2.30.2
>

jon June 5, 2023, 4:53 p.m. UTC | #2

Would flipping it around hurt less?

(I’d don’t speak perl but I think this would work!)


```
if ( &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
	$errormessage = "$Lang::tr{'extrahd cant umount'} $extrahdsettings{'PATH'}$Lang::tr{'extrahd maybe the device is in use'}?";
} else {
	open( FILE, "< $devicefile" ) or die "Unable to read $devicefile";
	@tmp = <FILE>;
	close FILE;
	open( FILE, "> $devicefile" ) or die "Unable to write $devicefile";
	foreach $deviceentry (sort @tmp)
	{
		@tmpline = split( /\;/, $deviceentry );
		if ( $tmpline[2] ne $extrahdsettings{'PATH'} )
		{
			print FILE $deviceentry;
		}
	}
	close FILE;
}
```

Jon Murphy
jon.murphy@ipfire.org



> On Jun 5, 2023, at 5:42 AM, Michael Tremer <michael.tremer@ipfire.org> wrote:
> 
> Reviewed-by: Michael Tremer <michael.tremer@ipfire.org>
> 
> The syntax pains me, but this is probably correct :)
> 
>> On 2 Jun 2023, at 20:01, Jon Murphy <jon.murphy@ipfire.org> wrote:
>> 
>> -Fixes remove entries in 'extrahd' via the webinterface for extrahd.cgi file.
>> 
>> Suggested-by: Bernhard Bitsch <bbitsch@ipfire.org>
>> Signed-off-by: Jon Murphy <jon.murphy@ipfire.org>
>> ---
>> html/cgi-bin/extrahd.cgi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/html/cgi-bin/extrahd.cgi b/html/cgi-bin/extrahd.cgi
>> index 36c0efc2e..9dd3a0eee 100644
>> --- a/html/cgi-bin/extrahd.cgi
>> +++ b/html/cgi-bin/extrahd.cgi
>> @@ -103,7 +103,7 @@ END
>> }
>> elsif ($extrahdsettings{'ACTION'} eq $Lang::tr{'delete'})
>> {
>> - if ( &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
>> + if ( ! &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
>> open( FILE, "< $devicefile" ) or die "Unable to read $devicefile";
>> @tmp = <FILE>;
>> close FILE;
>> -- 
>> 2.30.2
>> 
>

Michael Tremer June 7, 2023, 12:25 p.m. UTC | #3

Hello Jon,

> On 5 Jun 2023, at 17:53, jon <jon.murphy@ipfire.org> wrote:
> 
> Would flipping it around hurt less?
> 
> (I’d don’t speak perl but I think this would work!)

Oh no this wasn’t targeted at the code, just at Perl in general. I don’t find it obvious to read this at all, but there is no way to write this in an absolute intuitive way.

So I am happy to have this merged as it is. The most important thing is that it does the right thing.

-Michael

> ```
> if ( &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
> $errormessage = "$Lang::tr{'extrahd cant umount'} $extrahdsettings{'PATH'}$Lang::tr{'extrahd maybe the device is in use'}?";
> } else {
> open( FILE, "< $devicefile" ) or die "Unable to read $devicefile";
> @tmp = <FILE>;
> close FILE;
> open( FILE, "> $devicefile" ) or die "Unable to write $devicefile";
> foreach $deviceentry (sort @tmp)
> {
> @tmpline = split( /\;/, $deviceentry );
> if ( $tmpline[2] ne $extrahdsettings{'PATH'} )
> {
> print FILE $deviceentry;
> }
> }
> close FILE;
> }
> ```
> 
> Jon Murphy
> jon.murphy@ipfire.org
> 
> 
> 
>> On Jun 5, 2023, at 5:42 AM, Michael Tremer <michael.tremer@ipfire.org> wrote:
>> 
>> Reviewed-by: Michael Tremer <michael.tremer@ipfire.org>
>> 
>> The syntax pains me, but this is probably correct :)
>> 
>>> On 2 Jun 2023, at 20:01, Jon Murphy <jon.murphy@ipfire.org> wrote:
>>> 
>>> -Fixes remove entries in 'extrahd' via the webinterface for extrahd.cgi file.
>>> 
>>> Suggested-by: Bernhard Bitsch <bbitsch@ipfire.org>
>>> Signed-off-by: Jon Murphy <jon.murphy@ipfire.org>
>>> ---
>>> html/cgi-bin/extrahd.cgi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/html/cgi-bin/extrahd.cgi b/html/cgi-bin/extrahd.cgi
>>> index 36c0efc2e..9dd3a0eee 100644
>>> --- a/html/cgi-bin/extrahd.cgi
>>> +++ b/html/cgi-bin/extrahd.cgi
>>> @@ -103,7 +103,7 @@ END
>>> }
>>> elsif ($extrahdsettings{'ACTION'} eq $Lang::tr{'delete'})
>>> {
>>> - if ( &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
>>> + if ( ! &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
>>> open( FILE, "< $devicefile" ) or die "Unable to read $devicefile";
>>> @tmp = <FILE>;
>>> close FILE;
>>> -- 
>>> 2.30.2
>>> 
>> 
>

Bernhard Bitsch June 7, 2023, 7:30 p.m. UTC | #4

Reviewed-by: Bernhard Bitsch <bbitsch@ipfire.org>

Less painful syntax could be:
+	unless ( 
&General::system("/usr/loca/bin/extrahdctrl","umount",$extrahdsettings{'PATH'}")) 
{
Am 02.06.2023 um 21:01 schrieb Jon Murphy:
> -Fixes remove entries in 'extrahd' via the webinterface for extrahd.cgi file.
> 
> Suggested-by: Bernhard Bitsch <bbitsch@ipfire.org>
> Signed-off-by: Jon Murphy <jon.murphy@ipfire.org>
> ---
>   html/cgi-bin/extrahd.cgi | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/html/cgi-bin/extrahd.cgi b/html/cgi-bin/extrahd.cgi
> index 36c0efc2e..9dd3a0eee 100644
> --- a/html/cgi-bin/extrahd.cgi
> +++ b/html/cgi-bin/extrahd.cgi
> @@ -103,7 +103,7 @@ END
>   }
>   elsif ($extrahdsettings{'ACTION'} eq $Lang::tr{'delete'})
>   {
> -	if ( &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
> +	if ( ! &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
>   		open( FILE, "< $devicefile" ) or die "Unable to read $devicefile";
>   		@tmp = <FILE>;
>   		close FILE;

diff mbox series

Patch

diff --git a/html/cgi-bin/extrahd.cgi b/html/cgi-bin/extrahd.cgi
index 36c0efc2e..9dd3a0eee 100644
--- a/html/cgi-bin/extrahd.cgi
+++ b/html/cgi-bin/extrahd.cgi
@@ -103,7 +103,7 @@  END
 }
 elsif ($extrahdsettings{'ACTION'} eq $Lang::tr{'delete'})
 {
-	if ( &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
+	if ( ! &General::system("/usr/local/bin/extrahdctrl", "umount", "$extrahdsettings{'PATH'}")) {
 		open( FILE, "< $devicefile" ) or die "Unable to read $devicefile";
 		@tmp = <FILE>;
 		close FILE;