Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 487758 - app-editors/mg-20130922 cannot save files (on some libcs).
Summary: app-editors/mg-20130922 cannot save files (on some libcs).
Status: RESOLVED UPSTREAM
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Emacs project
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-12 10:45 UTC by Marien Zwart (RETIRED)
Modified: 2013-10-22 09:12 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments
files/mg-20130922-dirname.patch (mg-20130922-dirname.patch,466 bytes, patch)
2013-10-12 12:36 UTC, Ulrich Müller
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marien Zwart (RETIRED) gentoo-dev 2013-10-12 10:45:52 UTC
First the bug:

$ echo hi > /tmp/wat
$ mg /tmp/wat
Make some change to the file, hit C-x C-s.
An error message appears: "Cannot open file for writing : Is a directory"
Try again (C-x C-s):
File has changed on disk since last save. Save anyway? (yes or no) yes
Backup error, save anyway? (yes or no) yes
Cannot open file for writing : Is a directory

The cause is this new code in file.c (function "writeout"), checking permissions on the directory the target file is in:

       dp = dirname(fn);

       if (stat(fn, &statbuf) == -1 && errno == ENOENT) {
               errno = 0;
               if (access(dp, W_OK) && errno == EACCES) {
                       ewprintf("Directory %s%s write-protected", dp,
                           (dp[0] == '/' && dp[1] == '\0') ? "" : "/");
                       return (FIOERR);
               } else if (errno == ENOENT) {
                        ewprintf("%s%s: no such directory", dp,
                            (dp[0] == '/' && dp[1] == '\0') ? "" : "/");
                       return (FIOERR);
               }
        }

dirname(3) cautions: "Both dirname() and basename() may modify the contents of path, so it may be desirable to pass a copy when calling one of these functions". What mg passes to dirname is the filename as stored on the buffer, which it then also uses as path to open to save the file. So the first save attempt fails because it tries to open /tmp instead of /tmp/wat, and updates the buffer's filename. Each subsequent attempt strips another path component, and the backup failure on the second attempt occurs because it's now trying to back up to /tmp~ instead of to /tmp/wat~.

file.c actually defines xdirname and xbasename to address this very problem (they copy their argument), but does not use them consistently.

I'd recommend masking this new version of mg, as being unable to save edited files renders it a bit useless. I've not yet reported this upstream, please let me know if you want me to do that or if you think this'll get fixed more expediently if you do so (I see you're already listed as "sending patches" on their homepage).
Comment 1 Marien Zwart (RETIRED) gentoo-dev 2013-10-12 10:56:21 UTC
I am a bad bugreporter and forgot to include emerge --info:

Portage 2.2.7 (default/linux/amd64/13.0/desktop/gnome, gcc-4.7.3, glibc-2.17, 3.11.4-gentoo-m10-no-ohci x86_64)
=================================================================
                         System Settings
=================================================================
System uname: Linux-3.11.4-gentoo-m10-no-ohci-x86_64-Intel-R-_Core-TM-_i7-3770_CPU_@_3.40GHz-with-gentoo-2.2
KiB Mem:    16318476 total,   1252976 free
KiB Swap:    2097148 total,   2097148 free
Timestamp of tree: Unknown
ld GNU ld (GNU Binutils) 2.23.2
ccache version 3.1.9 [disabled]
app-shells/bash:          4.2_p45
dev-java/java-config:     2.2.0
dev-lang/python:          2.7.5-r2, 3.3.2-r2
dev-util/ccache:          3.1.9-r2
dev-util/cmake:           2.8.11.2
dev-util/pkgconfig:       0.28
sys-apps/baselayout:      2.2
sys-apps/openrc:          0.12.2
sys-apps/sandbox:         2.6-r1
sys-devel/autoconf:       2.13, 2.69
sys-devel/automake:       1.11.6::marienz, 1.12.6, 1.14
sys-devel/binutils:       2.23.2
sys-devel/gcc:            4.7.3-r1, 4.8.1-r1
sys-devel/gcc-config:     1.8
sys-devel/libtool:        2.4.2
sys-devel/make:           3.82-r4
sys-kernel/linux-headers: 3.11 (virtual/os-headers)
sys-libs/glibc:           2.17
Repositories: gentoo marienz
ACCEPT_KEYWORDS="amd64 ~amd64"
ACCEPT_LICENSE="@FREE @BINARY-REDISTRIBUTABLE freedist free-noncomm AdobeFlash-11.x Google-TOS D1X CC-Sampling-Plus-1.0 unRAR MPEG-4 gSOAP repoze MSttfEULA AMD"
CBUILD="x86_64-pc-linux-gnu"
CFLAGS="-ggdb -O2 -pipe -march=native"
CHOST="x86_64-pc-linux-gnu"
CONFIG_PROTECT="/etc /usr/share/config /usr/share/gnupg/qualified.txt /var/lib/hsqldb"
CONFIG_PROTECT_MASK="/etc/ca-certificates.conf /etc/dconf /etc/env.d /etc/fonts/fonts.conf /etc/gconf /etc/gentoo-release /etc/revdep-rebuild /etc/sandbox.d /etc/terminfo /etc/texmf/language.dat.d /etc/texmf/language.def.d /etc/texmf/updmap.d /etc/texmf/web2c"
CXXFLAGS="-ggdb -O2 -pipe -march=native"
DISTDIR="/usr/portage/distfiles"
FCFLAGS="-O2 -pipe"
FEATURES="assume-digests binpkg-logs candy collision-protect compressdebug config-protect-if-modified distlocks ebuild-locks fixlafiles merge-sync multilib-strict news parallel-fetch preserve-libs protect-owned sandbox sfperms sign splitdebug strict test unknown-features-warn unmerge-logs unmerge-orphans userfetch userpriv usersandbox usersync xattr"
FFLAGS="-O2 -pipe"
GENTOO_MIRRORS="http://mirror.internode.on.net/pub/gentoo/"
LANG="en_US.UTF-8"
LDFLAGS="-Wl,--hash-style=gnu -Wl,-O1 -Wl,--as-needed"
MAKEOPTS="-j8"
PKGDIR="/var/tmp/packages"
PORTAGE_CONFIGROOT="/"
PORTAGE_RSYNC_OPTS="--recursive --links --safe-links --perms --times --omit-dir-times --compress --force --whole-file --delete --stats --human-readable --timeout=180 --exclude=/distfiles --exclude=/local --exclude=/packages"
PORTAGE_TMPDIR="/var/tmp"
PORTDIR="/srv/gentoo-x86"
PORTDIR_OVERLAY="/usr/local/portage/private"
USE="X a52 aac acl acpi adns alsa amd64 avahi avx bluetooth branding btrfs bzip2 cairo canberra caps cdda cdio cdr clang cli clutter colord corefonts crypt cups cxx dbus device-mapper distinct-l doc dos dot dri dts dvd dvdr dvi eds efi emacs emboss encode equalizer evo exif expat ffmpeg firefox flac fontconfig fortran fuse g3dvl gflags ghcbootstrap gif git gles2 gmp gnome gnome-keyring gnome-online-accounts gpg gstreamer gtk gtk3 gtkstyle http iconv icu idn imap inotify introspection ipv6 irc jpeg lame latex lcms libass libcaca libffi libkms libnotify libsecret libvisual llvm lua lzma mad maildir minizip mmx mmxext mng modules mp3 mp4 mpeg mtp mudflap multilib nautilus ncurses networkmanager nfsidmap nls nptl numpy objc offensive ogg opengl openmp opus pam pango pch pdf playlist png policykit postscript ppds preview-latex pulseaudio python python3 qt3support qt4 readline realtime sandbox scanner schroedinger semantic-desktop session sip smtp socialweb speex spell spice sqlite sse sse2 sse3 sse4_1 ssl ssse3 startup-notification svg symlink system-icu system-jpeg systemd theora tiff tokyocabinet toolkit-scroll-bars tools truetype udev udisks udisks2 unicode upower urwid usb vala valgrind vorbis vpx webkit wxwidgets x264 xattr xcb xcomposite xft xinerama xml xmp xorg xv xvfb xvmc zlib zsh-completion" ABI_X86="64" ALSA_CARDS="hda_intel" APACHE2_MODULES="authn_core authz_core socache_shmcb unixd actions alias auth_basic authn_alias authn_anon authn_dbm authn_default authn_file authz_dbm authz_default authz_groupfile authz_host authz_owner authz_user autoindex cache cgi cgid dav dav_fs dav_lock deflate dir disk_cache env expires ext_filter file_cache filter headers include info log_config logio mem_cache mime mime_magic negotiation rewrite setenvif speling status unique_id userdir usertrack vhost_alias" CALLIGRA_FEATURES="kexi words flow plan sheets stage tables krita karbon braindump author" CAMERAS="ptp2" COLLECTD_PLUGINS="df interface irq load memory rrdtool swap syslog" CURL_SSL="nss" DRACUT_MODULES="btrfs" ELIBC="glibc" GPSD_PROTOCOLS="ashtech aivdm earthmate evermore fv18 garmin garmintxt gpsclock itrax mtk3301 nmea ntrip navcom oceanserver oldstyle oncore rtcm104v2 rtcm104v3 sirf superstar2 timing tsip tripmate tnt ubx" INPUT_DEVICES="evdev" KERNEL="linux" LCD_DEVICES="bayrad cfontz cfontz633 glk hd44780 lb216 lcdm001 mtxorb ncurses text" LIBREOFFICE_EXTENSIONS="presenter-console presenter-minimizer" LINGUAS="en en_GB en_US nl fy fy_NL" OFFICE_IMPLEMENTATION="libreoffice" PHP_TARGETS="php5-5" PYTHON_SINGLE_TARGET="python2_7" PYTHON_TARGETS="python2_7 python3_3 pypy2_0" QEMU_SOFTMMU_TARGETS="x86_64" RUBY_TARGETS="ruby19 ruby20" SANE_BACKENDS="u12" USERLAND="GNU" VIDEO_CARDS="intel i965" XTABLES_ADDONS="quota2 psd pknock lscan length2 ipv4options ipset ipp2p iface geoip fuzzy condition tee tarpit sysrq steal rawnat logmark ipmark dhcpmac delude chaos account"
USE_PYTHON="2.7 3.3 2.7-pypy-2.0"
Unset:  CPPFLAGS, CTARGET, EMERGE_DEFAULT_OPTS, INSTALL_MASK, LC_ALL, PORTAGE_BUNZIP2_COMMAND, PORTAGE_COMPRESS, PORTAGE_COMPRESS_FLAGS, PORTAGE_RSYNC_EXTRA_OPTS, SYNC

=================================================================
                        Package Settings
=================================================================

app-editors/mg-20130922 was built with the following:
USE="-livecd"
CFLAGS="-ggdb"

(the problem also occurs with my regular CFLAGS, of course).
Comment 2 Ulrich Müller gentoo-dev 2013-10-12 12:36:30 UTC
Created attachment 360688 [details, diff]
files/mg-20130922-dirname.patch

(In reply to Marien Zwart from comment #0)

Thanks for the nice analysis pinpointing the exact problem. :-)
Does attached patch fix it for you?

> I've not yet reported this upstream, please let me know if you want me to do
> that or if you think this'll get fixed more expediently if you do so (I see
> you're already listed as "sending patches" on their homepage).

Can you please report this upstream? As the bug is present in the OpenBSD version (see http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/file.c), it should be reported to both <hboetes@gmail.com> and <bugs@openbsd.org>.
Comment 3 Ulrich Müller gentoo-dev 2013-10-12 12:38:43 UTC
(In reply to Ulrich Müller from comment #2)
> Can you please report this upstream?

And CC me, please.
Comment 4 Marien Zwart (RETIRED) gentoo-dev 2013-10-13 02:37:47 UTC
(In reply to Ulrich Müller from comment #2)
> Created attachment 360688 [details, diff] [details, diff]
> files/mg-20130922-dirname.patch
> 
> (In reply to Marien Zwart from comment #0)
> 
> Thanks for the nice analysis pinpointing the exact problem. :-)
> Does attached patch fix it for you?

It does.

The function "readin" in the same file has a similarly suspicious usage of dirname, but it looks like that one doesn't currently bite because of how that function is called (with a buffer that's not reused).

> > I've not yet reported this upstream, please let me know if you want me to do
> > that or if you think this'll get fixed more expediently if you do so (I see
> > you're already listed as "sending patches" on their homepage).
> 
> Can you please report this upstream? As the bug is present in the OpenBSD
> version (see http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/file.c),
> it should be reported to both <hboetes@gmail.com> and <bugs@openbsd.org>.

This may not be a bug in the OpenBSD version. dirname(3) in OpenBSD cvs (I don't have an OpenBSD install handy...) says "returns a pointer to internal static storage space that will be overwritten by subsequent calls (each function has its own separate storage)", which means it'll work there. It says you should care about "other vendors" modifying the string passed in if you care about portability, which seems to imply relying on the OpenBSD version of dirname not modifying its input is considered acceptable.

I'll therefore email just hboetes@ (with you CC'd) and ask him to figure out if this also needs fixing in the OpenBSD version of mg.
Comment 5 Ulrich Müller gentoo-dev 2013-10-13 07:42:49 UTC
(In reply to Marien Zwart from comment #4)
> The function "readin" in the same file has a similarly suspicious usage of
> dirname, but it looks like that one doesn't currently bite because of how
> that function is called (with a buffer that's not reused).

Are you sure that it's not reused? readin() has many callers, and the call from dorevert() in buffer.c looks suspicious. Therefore, I've added the same fix to readin(). Committed as -r1.

> I'll therefore email just hboetes@ (with you CC'd) and ask him to figure out
> if this also needs fixing in the OpenBSD version of mg.

Fine with me, thanks.
Comment 6 Marien Zwart (RETIRED) gentoo-dev 2013-10-14 09:48:00 UTC
(In reply to Ulrich Müller from comment #5)
> (In reply to Marien Zwart from comment #4)
> > The function "readin" in the same file has a similarly suspicious usage of
> > dirname, but it looks like that one doesn't currently bite because of how
> > that function is called (with a buffer that's not reused).
> 
> Are you sure that it's not reused? readin() has many callers, and the call
> from dorevert() in buffer.c looks suspicious. Therefore, I've added the same
> fix to readin(). Committed as -r1.

I wasn't paying enough attention and only checked file.c. Thanks for catching that. -r1 works.
Comment 7 Ulrich Müller gentoo-dev 2013-10-22 09:11:25 UTC
Fixed in upstream CVS:
http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/mg/file.c?rev=1.88