[libdvdcss-devel] [PATCH] Check the return values of write() calls.

[Diego Biurrun]

Fixes the following two warnings:
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
---
src/css.c | 8 ++++++--
src/libdvdcss.c | 6 +++++-
2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/css.c b/src/css.c
index 935b7b2..2d13d5e 100644
--- a/src/css.c
+++ b/src/css.c
@@ -266,13 +266,17 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
i_fd = open( dvdcss->psz_cachefile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- char psz_key[KEY_SIZE * 3 + 2];
+ ssize_t len = KEY_SIZE * 3 + 2;
+ char psz_key[len];

sprintf( psz_key, "%02x:%02x:%02x:%02x:%02x\r\n",
p_title_key[0], p_title_key[1], p_title_key[2],
p_title_key[3], p_title_key[4] );

- write( i_fd, psz_key, KEY_SIZE * 3 + 1 );
+ if( write( i_fd, psz_key, len - 1 ) < len - 1 )
+ {
+ return -1;
+ }
close( i_fd );
}
}
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index 2a1a5bb..c922bef 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -377,7 +377,11 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
i_fd = open( psz_tagfile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- write( i_fd, psz_tag, strlen(psz_tag) );
+ ssize_t len = strlen(psz_tag);
+ if( write( i_fd, psz_tag, len ) < len )
+ {
+ return NULL;
+ }
close( i_fd );
}
}

[Reimar Döffinger]

Post by Diego Biurrun
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’,
declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]

Uh, am I reading that right that you make the code fail playing the DVD just because we failed to write to the cache? (Plus I think you forgot to close the file)
I'm quite sure that the point of the warning is not to make people break their code.
Unfortunately ignoring the result is the only sensible thing I can think of.
I was thinking about suggesting to delete the presumably corrupted file, but it doesn't seem like a really good idea either, particularly since it might actually still be fine if write returns 0.
I guess printing a warning should be fine though.

[Diego Biurrun]

Post by Reimar Döffinger

Post by Diego Biurrun
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’,
declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]

Uh, am I reading that right that you make the code fail playing the
DVD just because we failed to write to the cache? (Plus I think you
forgot to close the file) I'm quite sure that the point of the warning
is not to make people break their code. Unfortunately ignoring the
result is the only sensible thing I can think of. I was thinking about
suggesting to delete the presumably corrupted file, but it doesn't
seem like a really good idea either, particularly since it might
actually still be fine if write returns 0. I guess printing a warning
should be fine though.

I see no better alternative than printing a warning either.
Then again, the whole cache directory tag thing looks like
another dead remnant from the days of yore.

Diego

[Diego Biurrun]

Fixes the following two warnings:
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
---

Not leaking fds anymore; just print a warning if writing the cache fails.

src/css.c | 9 +++++++--
src/libdvdcss.c | 7 ++++++-
2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/css.c b/src/css.c
index 935b7b2..fb45927 100644
--- a/src/css.c
+++ b/src/css.c
@@ -266,13 +266,18 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
i_fd = open( dvdcss->psz_cachefile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- char psz_key[KEY_SIZE * 3 + 2];
+ ssize_t len = KEY_SIZE * 3 + 2;
+ char psz_key[len];

sprintf( psz_key, "%02x:%02x:%02x:%02x:%02x\r\n",
p_title_key[0], p_title_key[1], p_title_key[2],
p_title_key[3], p_title_key[4] );

- write( i_fd, psz_key, KEY_SIZE * 3 + 1 );
+ if( write( i_fd, psz_key, len - 1 ) < len - 1 )
+ {
+ print_error( dvdcss,
+ "Error caching key on disk, continuing..\n" );
+ }
close( i_fd );
}
}
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index 2a1a5bb..f1fbd9d 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -377,7 +377,12 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
i_fd = open( psz_tagfile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- write( i_fd, psz_tag, strlen(psz_tag) );
+ ssize_t len = strlen(psz_tag);
+ if( write( i_fd, psz_tag, len ) < len )
+ {
+ print_error( dvdcss,
+ "Error writing cache directory tag, continuing..\n" );
+ }
close( i_fd );
}
}

[Ivan Kalvachev]

Post by Diego Biurrun
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
---
Not leaking fds anymore; just print a warning if writing the cache fails.
src/css.c | 9 +++++++--
src/libdvdcss.c | 7 ++++++-
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/css.c b/src/css.c
index 935b7b2..fb45927 100644
--- a/src/css.c
+++ b/src/css.c
@@ -266,13 +266,18 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
i_fd = open( dvdcss->psz_cachefile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- char psz_key[KEY_SIZE * 3 + 2];
+ ssize_t len = KEY_SIZE * 3 + 2;
+ char psz_key[len];

I think Mans got rid of all VLA in LibAV recently, strange to see you
adding them to dvdcss.

[Diego Elio Pettenò]

That is not a VLA: the len variable is all precalculated. You could
hint it better by declaring it a 'static const' .
Diego Elio Pettenò — Flameeyes

Post by Ivan Kalvachev

Post by Diego Biurrun
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared
with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
---
Not leaking fds anymore; just print a warning if writing the cache fails.
src/css.c | 9 +++++++--
src/libdvdcss.c | 7 ++++++-
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/css.c b/src/css.c
index 935b7b2..fb45927 100644
--- a/src/css.c
+++ b/src/css.c
@@ -266,13 +266,18 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
i_fd = open( dvdcss->psz_cachefile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- char psz_key[KEY_SIZE * 3 + 2];
+ ssize_t len = KEY_SIZE * 3 + 2;
+ char psz_key[len];

I think Mans got rid of all VLA in LibAV recently, strange to see you
adding them to dvdcss.
_______________________________________________
libdvdcss-devel mailing list
http://mailman.videolan.org/listinfo/libdvdcss-devel

[Diego Biurrun]

Fixes the following two warnings:
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
---

Now with static const len variable.

src/css.c | 9 +++++++--
src/libdvdcss.c | 7 ++++++-
2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/css.c b/src/css.c
index 935b7b2..839fa29 100644
--- a/src/css.c
+++ b/src/css.c
@@ -266,13 +266,18 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
i_fd = open( dvdcss->psz_cachefile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- char psz_key[KEY_SIZE * 3 + 2];
+ static const ssize_t len = KEY_SIZE * 3 + 1;
+ char psz_key[len + 1];

sprintf( psz_key, "%02x:%02x:%02x:%02x:%02x\r\n",
p_title_key[0], p_title_key[1], p_title_key[2],
p_title_key[3], p_title_key[4] );

- write( i_fd, psz_key, KEY_SIZE * 3 + 1 );
+ if( write( i_fd, psz_key, len ) < len )
+ {
+ print_error( dvdcss,
+ "Error caching key on disk, continuing..\n" );
+ }
close( i_fd );
}
}
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index 2a1a5bb..f1fbd9d 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -377,7 +377,12 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
i_fd = open( psz_tagfile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- write( i_fd, psz_tag, strlen(psz_tag) );
+ ssize_t len = strlen(psz_tag);
+ if( write( i_fd, psz_tag, len ) < len )
+ {
+ print_error( dvdcss,
+ "Error writing cache directory tag, continuing..\n" );
+ }
close( i_fd );
}
}

[Ivan Kalvachev]

Post by Diego Elio Pettenò
That is not a VLA: the len variable is all precalculated. You could
hint it better by declaring it a 'static const' .

For the compiler it is still VLA, because you provide the size with a
variable, even if that variable holds a constant value.

Libav configure adds -Werror=vla and it would cause error at this code.

[Diego Biurrun]

Fixes the following two warnings:
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
---

... Now without VLA ...

src/css.c | 6 +++++-
src/libdvdcss.c | 7 ++++++-
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/css.c b/src/css.c
index a8b8e0e..90fc395 100644
--- a/src/css.c
+++ b/src/css.c
@@ -274,7 +274,11 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
p_title_key[0], p_title_key[1], p_title_key[2],
p_title_key[3], p_title_key[4] );

- write( i_fd, psz_key, PSZ_KEY_SIZE + 1 )
+ write( i_fd, psz_key, PSZ_KEY_SIZE + 1 ) < PSZ_KEY_SIZE + 1 )
+ {
+ print_error( dvdcss,
+ "Error caching key on disk, continuing..\n" );
+ }
close( i_fd );
}
}
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index 2a1a5bb..f1fbd9d 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -377,7 +377,12 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
i_fd = open( psz_tagfile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- write( i_fd, psz_tag, strlen(psz_tag) );
+ ssize_t len = strlen(psz_tag);
+ if( write( i_fd, psz_tag, len ) < len )
+ {
+ print_error( dvdcss,
+ "Error writing cache directory tag, continuing..\n" );
+ }
close( i_fd );
}
}

[Jean-Baptiste Kempf]

Post by Diego Biurrun
+ write( i_fd, psz_key, PSZ_KEY_SIZE + 1 ) < PSZ_KEY_SIZE + 1 )

missing an if somewhere?

[Diego Biurrun]

Post by Jean-Baptiste Kempf

Post by Diego Biurrun
+ write( i_fd, psz_key, PSZ_KEY_SIZE + 1 ) < PSZ_KEY_SIZE + 1 )

missing an if somewhere?

Ugh, WTF happened there?

Diego

[Diego Biurrun]

Fixes the following two warnings:
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
---

Now without silly brown paper bag mistakes.

src/css.c | 6 +++++-
src/libdvdcss.c | 7 ++++++-
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/css.c b/src/css.c
index a8b8e0e..681bb2c 100644
--- a/src/css.c
+++ b/src/css.c
@@ -274,7 +274,11 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
p_title_key[0], p_title_key[1], p_title_key[2],
p_title_key[3], p_title_key[4] );

- write( i_fd, psz_key, PSZ_KEY_SIZE + 1 )
+ if(write( i_fd, psz_key, PSZ_KEY_SIZE + 1 ) < PSZ_KEY_SIZE + 1 )
+ {
+ print_error( dvdcss,
+ "Error caching key on disk, continuing..\n" );
+ }
close( i_fd );
}
}
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index 2a1a5bb..f1fbd9d 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -377,7 +377,12 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
i_fd = open( psz_tagfile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- write( i_fd, psz_tag, strlen(psz_tag) );
+ ssize_t len = strlen(psz_tag);
+ if( write( i_fd, psz_tag, len ) < len )
+ {
+ print_error( dvdcss,
+ "Error writing cache directory tag, continuing..\n" );
+ }
close( i_fd );
}
}

[Diego Biurrun]

---
src/css.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/css.c b/src/css.c
index 935b7b2..a8b8e0e 100644
--- a/src/css.c
+++ b/src/css.c
@@ -61,6 +61,8 @@
#include "ioctl.h"
#include "device.h"

+#define PSZ_KEY_SIZE (KEY_SIZE * 3)
+
/*****************************************************************************
* Local prototypes
*****************************************************************************/
@@ -217,12 +219,12 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )

if( i_fd >= 0 )
{
- char psz_key[KEY_SIZE * 3];
+ char psz_key[PSZ_KEY_SIZE];
unsigned int k0, k1, k2, k3, k4;

- psz_key[KEY_SIZE * 3 - 1] = '\0';
+ psz_key[PSZ_KEY_SIZE - 1] = '\0';

- if( read( i_fd, psz_key, KEY_SIZE * 3 - 1 ) == KEY_SIZE * 3 - 1
+ if( read( i_fd, psz_key, PSZ_KEY_SIZE - 1 ) == PSZ_KEY_SIZE - 1
&& sscanf( psz_key, "%x:%x:%x:%x:%x",
&k0, &k1, &k2, &k3, &k4 ) == 5 )
{
@@ -266,13 +268,13 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
i_fd = open( dvdcss->psz_cachefile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- char psz_key[KEY_SIZE * 3 + 2];
+ char psz_key[PSZ_KEY_SIZE + 2];

sprintf( psz_key, "%02x:%02x:%02x:%02x:%02x\r\n",
p_title_key[0], p_title_key[1], p_title_key[2],
p_title_key[3], p_title_key[4] );

- write( i_fd, psz_key, KEY_SIZE * 3 + 1 );
+ write( i_fd, psz_key, PSZ_KEY_SIZE + 1 )
close( i_fd );
}
}

[Jean-Baptiste Kempf]

Post by Diego Biurrun
---
src/css.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

Seems ok to me.

Best regards,

[Diego Biurrun]

libdvdcss | branch: master | Diego Biurrun <***@biurrun.de> | Fri Mar 15 15:53:40 2013 +0100| [3dc40990ab9c9da2e9ed728d540c312b78d8a6bc] | committer: Diego Biurrun

Check the return values of write() calls.

Fixes the following two warnings:
src/libdvdcss.c:380:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
src/css.c:275:18: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]

http://git.videolan.org/gitweb.cgi/libdvdcss.git/?a=commit;h=3dc40990ab9c9da2e9ed728d540c312b78d8a6bc

---

src/css.c | 6 +++++-
src/libdvdcss.c | 7 ++++++-
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/css.c b/src/css.c
index a8b8e0e..681bb2c 100644
--- a/src/css.c
+++ b/src/css.c
@@ -274,7 +274,11 @@ int _dvdcss_title ( dvdcss_t dvdcss, int i_block )
p_title_key[0], p_title_key[1], p_title_key[2],
p_title_key[3], p_title_key[4] );

- write( i_fd, psz_key, PSZ_KEY_SIZE + 1 )
+ if(write( i_fd, psz_key, PSZ_KEY_SIZE + 1 ) < PSZ_KEY_SIZE + 1 )
+ {
+ print_error( dvdcss,
+ "Error caching key on disk, continuing..\n" );
+ }
close( i_fd );
}
}
diff --git a/src/libdvdcss.c b/src/libdvdcss.c
index 2a1a5bb..f1fbd9d 100644
--- a/src/libdvdcss.c
+++ b/src/libdvdcss.c
@@ -377,7 +377,12 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
i_fd = open( psz_tagfile, O_RDWR|O_CREAT, 0644 );
if( i_fd >= 0 )
{
- write( i_fd, psz_tag, strlen(psz_tag) );
+ ssize_t len = strlen(psz_tag);
+ if( write( i_fd, psz_tag, len ) < len )
+ {
+ print_error( dvdcss,
+ "Error writing cache directory tag, continuing..\n" );
+ }
close( i_fd );
}
}