Bug 125579 - [PATCH] sysutils/vobcopy should not fail if /etc/mtab does not exist
Summary: [PATCH] sysutils/vobcopy should not fail if /etc/mtab does not exist
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Greg Larkin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-13 19:30 UTC by Fabian Keil
Modified: 2008-08-11 21:40 UTC (History)
0 users

See Also:


Attachments
file.diff (1.38 KB, patch)
2008-07-13 19:30 UTC, Fabian Keil
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fabian Keil 2008-07-13 19:30:09 UTC
On my system vobcopy fails to work if /etc/mtab doesn't exist.
"touch /etc/mtab" works around the problem but is hardly an
acceptable solution.

The real problem seems to be that the FreeBSD-specific code in
dvd.c's get_device() isn't active. It's wrapped in:
#if ( defined( USE_STATFS_FOR_DEV ) || defined( USE_STATVFS_FOR_DEV ) ) 
which is false.

The attached patch makes it true by defining USE_STATFS_FOR_DEV.


I'm wondering if the port's modifications have been send
upstream as suggested in: 
http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/dads-freedback.html

I get the impression that this is a regression that might
have been prevented if upstream was aware of the port's needs.

Fix: Patch attached with submission follows:
How-To-Repeat: fk@TP51 /tank/test $vobcopy -m -i /cdrom/
[Hint] You use -i. Normally this is not necessary, vobcopy finds the input dir by itself. This option is only there if vobcopy makes trouble.
[Hint] If vobcopy makes trouble, please mail me so that I can fix this (robos@muon.de). Thanks
Vobcopy 1.1.0 - GPL Copyright (c) 2001 - 2007 robos@muon.de
[Hint] All lines starting with "libdvdread:" are not from vobcopy but from the libdvdread-library
[Error] Could not read /etc/mtab!
[Error] error: No such file or directory
[Error] Could not get the device which belongs to the given path!
Comment 1 Edwin Groothuis freebsd_committer 2008-07-13 19:30:17 UTC
Maintainer of sysutils/vobcopy,

Please note that PR ports/125579 has just been submitted.

If it contains a patch for an upgrade, an enhancement or a bug fix
you agree on, reply to this email stating that you approve the patch
and a committer will take care of it.

The full text of the PR can be found at:
    http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/125579

-- 
Edwin Groothuis via the GNATS Auto Assign Tool
edwin@FreeBSD.org
Comment 2 Edwin Groothuis freebsd_committer 2008-07-13 19:30:19 UTC
State Changed
From-To: open->feedback

Awaiting maintainers feedback (via the GNATS Auto Assign Tool)
Comment 3 Josh Paetzel <josh@tcbug.org> 2008-07-24 16:23:31 UTC
On Sunday 13 July 2008 06:30:17 pm Edwin Groothuis wrote:
> Maintainer of sysutils/vobcopy,
>
> Please note that PR ports/125579 has just been submitted.
>
> If it contains a patch for an upgrade, an enhancement or a bug fix
> you agree on, reply to this email stating that you approve the patch
> and a committer will take care of it.
>
> The full text of the PR can be found at:
>     http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/125579


This patch looks good.  Please commit it.  I submitted the patches to the 
upstream maintainers as well.

-- 
Thanks,

Josh Paetzel

PGP: 8A48 EF36 5E9F 4EDA 5A8C 11B4 26F9 01F1 27AF AECB
Comment 4 Mark Linimon freebsd_committer 2008-07-24 21:56:37 UTC
State Changed
From-To: feedback->open

Maintainer approved.
Comment 5 Greg Larkin freebsd_committer 2008-07-25 22:31:17 UTC
Responsible Changed
From-To: freebsd-ports-bugs->glarkin

I'll take it.
Comment 6 Greg Larkin freebsd_committer 2008-07-30 22:22:10 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi everyone,

I've been working on this PR, and I've run into a couple of issues.
First, part of the post-patch target wasn't working for me, and the
/etc/mtab file reference in dvd.c was not updated to /etc/fstab.  That
caused the "/etc/mtab is missing" message to appear when running vobcopy.

I made this change to the port Makefile to fix that:


~ post-patch:
- -       @${REINPLACE_CMD} -E -e 's|(fopen\("/etc/)mtab|\1fstab|; \
+       @${REINPLACE_CMD} -E -e 's|(fopen\( *"/etc/)mtab|\1fstab|; \
~                s|iso9660|cd9660|' ${WRKSRC}/dvd.c


I just loosened up the matching on the fopen function call to account
for whitespace after the open paren.

Once I did that, vobcopy no longer complained about the /etc/mtab file,
but it then core dumped.  This appears to be a return value checking
and/or parsing problem on line 513 of dvd.c:

513:       k = strstr( tmp_bufferin, " " );

~           /*traverse the gap*/

517:       if( isgraph( (int) *(k) ))
~             k++;
~           while(!(isgraph( (int) *(k) )))
~             k++;


The problem is that on my machine, /etc/fstab has tab characters between
the fields instead of spaces.  The strstr() call above returns 0, and
the isgraph call derefs a NULL pointer.

Should this problem be fixed in the upstream package before I go any
further with the PR?  Has anyone else run into this problem or the
earlier issue with the Makefile?

Best regards,
Greg
- --
Greg Larkin
http://www.sourcehosting.net/
http://www.FreeBSD.org/ - The Power To Serve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIkNuC0sRouByUApARAlfTAJ9OXzIO7imtaWkSbKI/AN7PqAAg9gCgy528
TbpG0i3XspVgvAYSYcsC3t0=
=v050
-----END PGP SIGNATURE-----
Comment 7 Josh Paetzel <josh@tcbug.org> 2008-07-31 08:35:51 UTC
On Wednesday 30 July 2008 09:22:10 pm Greg Larkin wrote:
> Hi everyone,
>
> I've been working on this PR, and I've run into a couple of issues.
> First, part of the post-patch target wasn't working for me, and the
> /etc/mtab file reference in dvd.c was not updated to /etc/fstab.  That
> caused the "/etc/mtab is missing" message to appear when running vobcopy.
>
> I made this change to the port Makefile to fix that:
>
>
> ~ post-patch:
> -       @${REINPLACE_CMD} -E -e 's|(fopen\("/etc/)mtab|\1fstab|; \
> +       @${REINPLACE_CMD} -E -e 's|(fopen\( *"/etc/)mtab|\1fstab|; \
> ~                s|iso9660|cd9660|' ${WRKSRC}/dvd.c
>
>
> I just loosened up the matching on the fopen function call to account
> for whitespace after the open paren.
>
> Once I did that, vobcopy no longer complained about the /etc/mtab file,
> but it then core dumped.  This appears to be a return value checking
> and/or parsing problem on line 513 of dvd.c:
>
> 513:       k = strstr( tmp_bufferin, " " );
>
> ~           /*traverse the gap*/
>
> 517:       if( isgraph( (int) *(k) ))
> ~             k++;
> ~           while(!(isgraph( (int) *(k) )))
> ~             k++;
>
>
> The problem is that on my machine, /etc/fstab has tab characters between
> the fields instead of spaces.  The strstr() call above returns 0, and
> the isgraph call derefs a NULL pointer.
>
> Should this problem be fixed in the upstream package before I go any
> further with the PR?  Has anyone else run into this problem or the
> earlier issue with the Makefile?
>
> Best regards,
> Greg


I've sent numerous emails to the upstream "maintainer" over the years and 
never gotten a reply.  Given that the last release was really done by someone 
other than the author I suspect he wants to wash his hands of it completely.  
It's also possible he wishes that vobcopy would go away, or that he has 
something against ! linux.

My recommendation is to fix the problem in the port, I have concidered vobcopy 
abandonware for years.


-- 
Thanks,

Josh Paetzel

PGP: 8A48 EF36 5E9F 4EDA 5A8C 11B4 26F9 01F1 27AF AECB
Comment 8 Josh Paetzel <josh@tcbug.org> 2008-07-31 21:49:27 UTC
On Friday 01 August 2008 12:38:03 am Greg Larkin wrote:
> Josh Paetzel wrote:
> | I've sent numerous emails to the upstream "maintainer" over the years and
> | never gotten a reply.  Given that the last release was really done by
>
> someone
>
> | other than the author I suspect he wants to wash his hands of it
>
> completely.
>
> | It's also possible he wishes that vobcopy would go away, or that he has
> | something against ! linux.
> |
> | My recommendation is to fix the problem in the port, I have concidered
>
> vobcopy
>
> | abandonware for years.
>
> Hi Josh,
>
> Do you mean fix the problem in the source code?  If I commit my changes
> to the Makefile, the program will still core dump without some bug
> fixing in the source.
>
> For now, I think the next step is to mark this port as BROKEN until a
> new source code release appears or someone creates a patch that gets it
> working on FreeBSD.  Let me know how that sounds when you have a chance.
>
> Thank you,
> Greg


Well, it seems to work fine for me, and I haven't been getting reports of 
widespread disfunction from anyone.  How can I reproduce this coredump?

I don't believe there will be any fixes from upstream.  I've never been able 
to contact the author at all.  Not sure if this is because of abandonment or 
he simply doesn't want to deal with ! linux.

-- 
Thanks,

Josh Paetzel

PGP: 8A48 EF36 5E9F 4EDA 5A8C 11B4 26F9 01F1 27AF AECB
Comment 9 Greg Larkin freebsd_committer 2008-08-01 01:38:03 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Josh Paetzel wrote:

|
| I've sent numerous emails to the upstream "maintainer" over the years and
| never gotten a reply.  Given that the last release was really done by
someone
| other than the author I suspect he wants to wash his hands of it
completely.
| It's also possible he wishes that vobcopy would go away, or that he has
| something against ! linux.
|
| My recommendation is to fix the problem in the port, I have concidered
vobcopy
| abandonware for years.
|
|

Hi Josh,

Do you mean fix the problem in the source code?  If I commit my changes
to the Makefile, the program will still core dump without some bug
fixing in the source.

For now, I think the next step is to mark this port as BROKEN until a
new source code release appears or someone creates a patch that gets it
working on FreeBSD.  Let me know how that sounds when you have a chance.

Thank you,
Greg
- --
Greg Larkin
http://www.sourcehosting.net/
http://www.FreeBSD.org/ - The Power To Serve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIklrr0sRouByUApARAm7vAJ9zDga97iZD2k6nRtQ/rB2lT1BJlwCgijUG
iVUrmTUmJIAfJ0WXyurjb58=
=Xwgb
-----END PGP SIGNATURE-----
Comment 10 Josh Paetzel <josh@tcbug.org> 2008-08-01 11:52:04 UTC
On Friday 01 August 2008 03:38:01 pm Greg Larkin wrote:
> Josh Paetzel wrote:
> | Well, it seems to work fine for me, and I haven't been getting reports of
> | widespread disfunction from anyone.  How can I reproduce this coredump?
> |
> | I don't believe there will be any fixes from upstream.  I've never
>
> been able
>
> | to contact the author at all.  Not sure if this is because of
>
> abandonment or
>
> | he simply doesn't want to deal with ! linux.
>
> Hi Josh,
>
> The program core dumped for me because my /etc/fstab uses tabs instead
> of spaces as it delimiters.  I changed the tabs to spaces, and it worked
> fine.  I would be interested to know what delimiter characters are in
> your /etc/fstab and whether you see any problems with tabs.
>
> I haven't done any C programming in many years, but this specific bug
> was easy to fix:
>
> --- dvd.c.orig  Tue Dec  5 00:06:52 2006
> +++ dvd.c       Tue Dec  5 00:09:54 2006
> @@ -510,7 +510,7 @@
> ~           */
>
>
> -          k = strstr( tmp_bufferin, " " );
> +          k = strpbrk( tmp_bufferin, " \t" );
>
> ~           /*traverse the gap*/
>
>
> I added that patch to files/patch-dvd.c in the port, and I'll submit the
> diffs to my mentor for commit approval.
>
> Best regards,
> Greg


My /etc/fstab uses tabs as delimiters.  Could you please provide me with an 
example usage so I can test it locally?

Doesn't that patch simply change the problem from fstabs with tabs to fstabs 
with spaces?

-- 
Thanks,

Josh Paetzel

PGP: 8A48 EF36 5E9F 4EDA 5A8C 11B4 26F9 01F1 27AF AECB
Comment 11 Greg Larkin freebsd_committer 2008-08-01 16:38:01 UTC
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Josh Paetzel wrote:

|
| Well, it seems to work fine for me, and I haven't been getting reports of
| widespread disfunction from anyone.  How can I reproduce this coredump?
|
| I don't believe there will be any fixes from upstream.  I've never
been able
| to contact the author at all.  Not sure if this is because of
abandonment or
| he simply doesn't want to deal with ! linux.
|

Hi Josh,

The program core dumped for me because my /etc/fstab uses tabs instead
of spaces as it delimiters.  I changed the tabs to spaces, and it worked
fine.  I would be interested to know what delimiter characters are in
your /etc/fstab and whether you see any problems with tabs.

I haven't done any C programming in many years, but this specific bug
was easy to fix:

- --- dvd.c.orig  Tue Dec  5 00:06:52 2006
+++ dvd.c       Tue Dec  5 00:09:54 2006
@@ -510,7 +510,7 @@
~           */


- -          k = strstr( tmp_bufferin, " " );
+          k = strpbrk( tmp_bufferin, " \t" );

~           /*traverse the gap*/


I added that patch to files/patch-dvd.c in the port, and I'll submit the
diffs to my mentor for commit approval.

Best regards,
Greg
- --
Greg Larkin
http://www.sourcehosting.net/
http://www.FreeBSD.org/ - The Power To Serve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIky3Z0sRouByUApARAjGCAJ9B4wWYMgASAIHD4zUOVjXjB+WqgACffpgj
+VtLA5cYOcMIoMHMb91rDeE=
=p4Pu
-----END PGP SIGNATURE-----
Comment 12 dfilter service freebsd_committer 2008-08-11 21:34:43 UTC
glarkin     2008-08-11 20:34:27 UTC

  FreeBSD ports repository

  Modified files:
    sysutils/vobcopy     Makefile 
    sysutils/vobcopy/files patch-dvd.c patch-vobcopy.h 
  Log:
  - Fixed broken REINPLACE_CMD regexp in Makefile
  - Fixed dvd.c patch file to support FreeBSD /etc/fstab delimiters and
    avoid core dump
  - Minor stylistic fixes in Makefile
  
  PR:             ports/125579
  Submitted by:   Fabian Keil <fk@fabiankeil.de>
  Reviewed by:    Josh Paetzel <josh@tcbug.org> (maintainer)
  Approved by:    beech (mentor, implicit)
  
  Revision  Changes    Path
  1.29      +6 -5      ports/sysutils/vobcopy/Makefile
  1.3       +20 -3     ports/sysutils/vobcopy/files/patch-dvd.c
  1.5       +4 -3      ports/sysutils/vobcopy/files/patch-vobcopy.h
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 13 Greg Larkin freebsd_committer 2008-08-11 21:34:45 UTC
State Changed
From-To: open->closed

Committed with modifications, thanks!