Skip to main content

the avatar of Joe Shaw

Don't defer Close() on writable files

Update: Another approach suggested by the inimitable Ben Johnson has been added to the end of the post.
Update 2: Discussion about fsync() added to the end of the post.

It’s an idiom that quickly becomes rote to Go programmers: whenever you conjure up a value that implements the io.Closer interface, after checking for errors you immediately defer its Close() method. You see this most often when making HTTP requests:

resp, err := http.Get("https://joeshaw.org")
if err != nil {
    return err
}
defer resp.Body.Close()

or opening files:

f, err := os.Open("/home/joeshaw/notes.txt")
if err != nil {
    return err
}
defer f.Close()

But this idiom is actually harmful for writable files because deferring a function call ignores its return value, and the Close() method can return errors. For writable files, Go programmers should avoid the defer idiom or very infrequent, maddening bugs will occur.

Why would you get an error from Close() but not an earlier Write() call? To answer that we need to take a brief, high-level detour into the area of computer architecture.

Generally speaking, as you move outside and away from your CPU, actions get orders of magnitude slower. Writing to a CPU register is very fast. Accessing system RAM is quite slow in comparison. Doing I/O on disks or networks is an eternity.

If every Write() call committed the data to the disk synchronously, the performance of our systems would be unusably slow. While synchronous writes are very important for certain types of software (like databases), most of the time it’s overkill.

The pathological case is writing to a file one byte at a time. Hard drives – brutish, mechanical devices – need to physically move a magnetic head to the position on the platter and possibly wait for a full platter revolution before the data could be persisted. SSDs, which store data in blocks and have a finite number of write cycles for each block, would quickly burn out as blocks are repeatedly written and overwritten.

Fortunately this doesn’t happen because multiple layers within hardware and software implement caching and write buffering. When you call Write(), your data is not immediately being written to media. The operating system, storage controllers and the media itself are all buffering the data in order to batch smaller writes together, organizing the data optimally for storage on the medium, and deciding when best to commit it. This turns our writes from slow, blocking synchronous operations to quick, asynchronous operations that don’t directly touch the much slower I/O device. Writing a byte at a time is never the most efficient thing to do, but at least we are not wearing out our hardware if we do it.

Of course, the bytes do have to be committed to disk at some point. The operating system knows that when we close a file, we are finished with it and no subsequent write operations are going to happen. It also knows that closing the file is its last chance to tell us something went wrong.

On POSIX systems like Linux and macOS, closing a file is handled by the close system call. The BSD man page for close(2) talks about the errors it can return:

ERRORS
     The close() system call will fail if:

     [EBADF]            fildes is not a valid, active file descriptor.

     [EINTR]            Its execution was interrupted by a signal.

     [EIO]              A previously-uncommitted write(2) encountered an input/output
                        error.

EIO is exactly the error we are worried about. It means that we’ve lost data trying to save it to disk, and our Go programs should absolutely not return a nil error in that case.

The simplest way to solve this is simply not to use defer when writing files:

func helloNotes() error {
    f, err := os.Create("/home/joeshaw/notes.txt")
    if err != nil {
        return err
    }

    if err = io.WriteString(f, "hello world"); err != nil {
        f.Close()
        return err
    }

    return f.Close()
}

This does mean additional bookkeeping of the file in the case of errors: we must explicitly close it in the case where io.WriteString() fails (and ignore its error, because the write error takes precedence). But it’s clear, straightforward, and properly checks the error from the f.Close() call.

There is a way to handle this case with defer by using named return values and a closure:

func helloNotes() (err error) {
    var f *os.File
    f, err = os.Create("/home/joeshaw/notes.txt")
    if err != nil {
        return
    }

    defer func() {
        cerr := f.Close()
        if err == nil {
            err = cerr
        }
    }()

    err = io.WriteString(f, "hello world")
    return
}

The main benefit of this pattern is that it’s not possible to forget to close the file because the deferred closure always executes. In longer functions with more if err != nil conditional branches, this pattern can also result in fewer lines of code and less repetition.

Still, I find this pattern to be a little too magical. I dislike using named return values, and modifying the return value after the core function finishes is not intuitively clear even to experienced Go programmers.

I am willing to accept the tradeoff of more readable and easily understandable code for needing to obsessively review code to ensure that the file is closed in all cases, and that’s the approach I recommend in code reviews I give to others.

Update

On Twitter, Ben Johnson suggested that Close() may be safe to run multiple times on files, like so:

func doSomething() error {
    f, err := os.Create("foo")
    if err != nil {
        return err
    }
    defer f.Close()

    if _, err := f.Write([]byte("bar"); err != nil {
        return err
    }

    if err := f.Close(); err != nil {
        return err
    }
    return nil
}

(gist)

The Go docs on io.Closer explicitly say that at an interface level behavior after the first call is unspecificed, but specific implementations may document their own behavior.

The docs for *os.File unfortunately aren’t clear on its behavior, saying only, “Close closes the File, rendering it unusable for I/O. It returns an error, if any.” The implemenation as of 1.8, however, shows:

func (f *File) Close() error {
    if f == nil {
        return ErrInvalid
    }
    return f.file.close()
}

func (file *file) close() error {
    if file == nil || file.fd == badFd {
        return syscall.EINVAL
    }
    var err error
    if e := syscall.Close(file.fd); e != nil {
        err = &PathError{"close", file.name, e}
    }
    file.fd = -1 // so it can't be closed again

    // no need for a finalizer anymore
    runtime.SetFinalizer(file, nil)
    return err
}

For clarity, badFd is defined as -1, so subsequent attempts to close an *os.File will do nothing and return syscall.EINVAL. But since we are ignoring the error from the defer, this doesn’t matter. It’s not idempotent, exactly, but as Ben put later in the Twitter thread, it “won’t blow shit up if you call it twice.”

The implementation is a good, common-sense one and it seems unlikely to change in the future and cause problems. But the lack of documentation about this outcome makes me a little nervous. Maybe a doc update to codify this behavior would be a good task for Go 1.10.

Update 2

Closing the file is the last chance the OS has to tell us about problems, but the buffers are not necessarily going to be flushed when you close the file. It’s entirely possible that flushing the write buffer to disk will happen after you close the file, and a failure there cannot be caught. If this happens, it usually means you have something seriously wrong, like a failing disk.

However, you can force the write to disk with the Sync() method on *os.File, which calls the fsync system call. You should check for errors from that call, but then I think it’s safe to ignore an error from Close(). Calling fsync has serious implications on performance: it’s flushing write buffers out to slow disks. But if you really, really want the data on disk, the best pattern to follow is probably:

func helloNotes() error {
    f, err := os.Create("/home/joeshaw/notes.txt")
    if err != nil {
        return err
    }
    defer f.Close()

    if err = io.WriteString(f, "hello world"); err != nil {
        return err
    }

    return f.Sync()
}

the avatar of Frédéric Crozat

Synology PhotoStation password vulnerability

On Synology NAS, synophoto_dsm_user executable, part of PhotoStation package, was leaking NAS user password on the command line.

Using a simple shell loop to run "ps ax | grep synophoto_dsm_user", it was possible to get user and password credentials for user on the NAS who had PhotoStation enabled with their DSM credentials.

Fortunately, by default, shell access on the NAS is not available (by ssh or telnet), it has to be enabled by the admin.

Still, it is a bad practise to pass credentials to process using command line, which can be intercepted.

PhotoStation version 6.7.1-3419 or earlier is vulnerable. I've contacted Synology and they should release a security fix really shortly, as well as a CVE for it.

Update (June 13, 2017): Synology has released a CVE and the vulnerability is fixed in PhotoStation 6.7.2-3429 or later. Remember to update this package on your NAS !
the avatar of Federico Mena-Quintero

Moving to a new blog engine

In 2003 I wrote an Emacs script to write my blog and produce an RSS feed. Back then, I seemed to write multiple short blog entries in a day rather than longer articles (doing Mastodon before it was cool?). But my blogging patterns have changed. I've been wanting to add some more features to the script: moving to a page-per-post model, support for draft articles, tags, and syntax highlighting for code excerpts...

This is a wheel that I do not find worth reinventing these days. After asking on Mastodon about static site generators (thanks to everyone who replied!), I've decided to give Pelican a try. I've reached the age where "obvious, beautiful documentation" is high on my list of things to look for when shopping for tools, and Pelican's docs are nice from the start.

The old blog is still available in the old location.

If you find broken links, or stuff that doesn't work correctly here, please mail me!

the avatar of Efstathios Iosifidis

Install Nextcloud client for openSUSE, Arch Linux, Fedora, Ubuntu based, Debian, Android, iOS

You have your Nextcloud instance installed. But how can you sync files from your computer to your server? You need a client on your desktop-laptop. We will see how your can install desktop client for openSUSE, Arch Linux, Fedora, Ubuntu based distros and of course you can see the links to mobile clients.

ANDROID/iOS
Let's start with the easy ones, the mobile clients.
Download for Android or for iOS (iOS costs $0.99).

GNOME
You don't need to install anything if you have GNOME version 3.24. You can go to SYSTEM SETTINGS>ONLINE ACCOUNTS and you'll see Nextcloud option.


You add your server and username/password.


And you can see a bookmark on Nautilus (Files). This option doesn't save anything on your local disk. It uploads the files to your instance. So it might be a little slow (depends on your bandwith).

If you have older version of GNOME, you can use ownCloud option. It works fine with your Nextcloud instance.

openSUSE
openSUSE has nextcloud client in the repositories.


If you cannot find the files, you can search for the packages here and use 1 click install.

Here is how you can install the client

# If you have GNOME
zypper in nautilus-extension-nextcloud nextcloud-client

# If you have MATE
zypper in caja-extension-nextcloud nextcloud-client

# If you have Cinnamon
zypper in nemo-extension-nextcloud nextcloud-client

# If you have KDE
zypper in nextcloud-client-dolphin nextcloud-client

Arch Linux
The client package is in AUR repository. You can read Arch Wiki for more information.

yaourt -S nextcloud-client

Fedora
The client is available for all versions and architectures. You can find more about the package here:

https://apps.fedoraproject.org/packages/nextcloud-client/

You can install it using the command

dnf install nextcloud-client

or download the rpm and install it.

Ubuntu based distros
The client is available in Launchpad.

To install it, open your terminal and use the following commands:

sudo -s
add-apt-repository ppa:nextcloud-devs/client
apt update
apt install nextcloud-client

Debian
You need to add to sources.list (nano /etc/apt/sources.list) one of the source lines below corresponding to your Debian version:

deb http://download.opensuse.org/repositories/home:/ivaradi/Debian_9.0_update/ /
deb http://download.opensuse.org/repositories/home:/ivaradi/Debian_9.0/ /
deb http://download.opensuse.org/repositories/home:/ivaradi/Debian_8.0/ /
deb http://download.opensuse.org/repositories/home:/ivaradi/Debian_7.0/ /

Before installing, you also need to add the respository's key to the list of trusted APT keys with a command line:

wget -q -O - /Release.key | apt-key add -y

For example (as root):

echo 'deb http://download.opensuse.org/repositories/home:/ivaradi/Debian_9.0/ /' > /etc/apt/sources.list.d/nextcloud-client.list
wget -q -O - http://download.opensuse.org/repositories/home:/ivaradi/Debian_9.0/Release.key | apt-key add -
apt-get update
apt-get install nextcloud-client

SETUP THE CLIENT
Next step is to configure the client. It's very easy.

First of all enter the URL for your instance.

Then enter username and password.

Then configure what to be synced. Press connect.

And when everything is OK press Finish.

If you want to check the client properties, click on the cloud icon. There, you can add a second account, maybe from another Nextcloud instance.

the avatar of Bernhard M. Wiedemann

The issues with contributing to projects only once

I work to improve the openSUSE Tumbleweed (GNU/)Linux distribution. Specifically I make sure that all packages can be built twice on different hosts and still produce identical results, which has multiple benefits. This generates a lot of patches in a single week.

OBS
Sometimes it is enough to adjust the .spec file – that is a small text file usually specific to us. Then it is straight-forward

  1. osc bco
  2. cd $PROJECT/$PACKAGE
  3. optional: spec_add_patch $MY.patch $SOME.spec
  4. edit *.spec
  5. osc build
  6. osc vc
  7. osc ci
  8. osc sr

And OBS will even auto-clean the branch when the submit-request is accepted. And it has a ‘tasks’ page to see and track SRs in various stages. For the spec_add_patch to work, you need to do once
ln -s /usr/lib/build/spec_add_patch /usr/local/bin/

When you want to contribute patches upstream, so that other distributions benefit from your improvements as well, then you first need to find out, where they collaborate. A good starting point is the URL field in the .spec file, but a google search for ‘contribute $PROJECT’ often is better.

github
Then there are those many projects hosted on github, where it is also pretty low effort, because I already have the account and it even remains signed in. But some repos on github are only read-only mirrors.

  1. check pull requests, if some have been merged recently
  2. fork the project
  3. git clone git@github.com:…
  4. cd $REPO
  5. edit $FILES
  6. git commit -a
  7. git push
  8. open pull request
  9. maybe have to sign a CLA for the project
  10. When change is accepted, delete fork to not clutter up repository list too much (on github under settings)

sourceforge
The older brother of github. They integrate various ways of contributing. The easiest one is to open a Ticket (Patch or Bug) and attach the .patch you want them to merge with a good description. While many developers do not have the time and energy to debug every bug you file, applying patches is much easier, so gets your issue fixed with a higher chance.

devel Mailinglist
Some projects collaborate mainly through their development MLs, then I need to

  1. subscribe
  2. confirm the subscription
  3. git format-patch origin/master
  4. git send-email –to $FOO-devel@… –from $MYSUBSCRIBEDEMAIL 000*.patch
  5. wait for replies
  6. if it is a high-volume ML, also add an IMAP folder and an entry to .procmailrc
  7. unsubscribe
  8. confirm

project bugtracker
Like https://bugzilla.gnome.org/ https://bugs.python.org/ https://bugs.ruby-lang.org/ https://bz.apache.org/bugzilla/

  1. create unique email addr
  2. sign up for account
  3. add info to my account list
  4. optional: search for existing bug (90% of the time there is none)
  5. file bug
  6. attach patch

So as you can see there is a wide range of ways. And most of them have some initial effort that you would only have once… But then I only contribute once per project, so I always pay that.

Thus, please make it easy for people to contribute one simple fix.

the avatar of Frank Karlitschek

Is doing nothing evil?

Last weekend I attended the openSUSE conference in Nürnberg. This is a really nice conference. Awesome location, great people, and an overall very relaxed atmosphere. I gave a talk about Nextcloud Security and what we plan to do in the future to make hosting a Nextcloud instance even easier and more secure.

I attended the Saturday keynote which triggered a reaction on my side that I wanted to share. This is only my personal opinion and I’m sure a lot of people think differently. But I can’t help my self.

The talk was about management of infrastructure and automation. It was a really good talk from a technical perspective. Very informative and detailed. But in the middle of the talk, the presenter mentioned that he was involved in building autonomous military submarines. This is of course controversial. I personally wouldn’t get involved in actual weapon development, building things which sole purpose is to kill people. But I understand that people have different opinions here and I can live with such a disagreement.

However, a bit later the presenter mentioned that he also worked for the US intelligence community to build surveillance systems to spy on people on a mass scale. Global, mass scale surveillance, which obviously involves all the people in the room. Which he pointed out as a some kind of joke, noting he might have helped spy on the people in the room.

I’m sorry but I don’t think this is funny at all. The global surveillance systems are undemocratic, in a lot of cases illegal and an attack on the basic human rights of people.

I understand that playing and working with cool technology is fun. And there is a lot of opportunity to do this for secret services and for the military to earn money. But I think we as software developers have a responsibility here. We are building the technology of the future. So we as developers are partly in control of how the world looks like in a few years.

We can’t just say: I close my eyes because this is only a job. Or I don’t want to know how this technology is used. I didn’t ask and no one told me so I’m innocent and not involved. Let me quote a well known saying here: “All that is necessary for the triumph of evil is that good men do nothing.”

I really have a hard time accepting that some people think that building mass surveillance systems is somehow funny or cool. And it is even more troubling to tell this the people your helped put under surveillance into their face and think that this is fun.

Sorry for the rant. But technology matter. Developers matter. Software matters and can be used in good ways and bad ways. We as developers and free software community have a responsibility and should not close our eyes.

a silhouette of a person's head and shoulders, used as a default avatar

LMBench versus GCC Optimisations

A quality benchmark is authoritative and trustworthy, and when you’re using one it’s a bit like playing the card game Snap: the rules are easy, and when the game is over it’s obvious who won.

But a poor benchmark makes performance work more like trying to solve a twisted version of the Knights and Knaves riddle where you’re not sure if the answers you’re getting are truths or lies, no one ever wins, and you only stop playing because you’re exhausted.

LMBench definitely has that riddle vibe.

I just don’t trust the test results that it spits out because I’ve run into too many dead ends when investigating performance issues that turned out to be false positives. And if there’s one thing that you need to be sure of when measuring performance, it’s the accuracy of your results.

So I was less than convinced when I recently saw that the int64-mul subtest of the LMBench ops microbenchmark was taking between 10% and 20% longer to run with a new enterprise kernel.

With my suspicions suitably heightened, I started reading the source code to understand exactly what the test was doing.

The int64-mul subtest tests the CPU speed of 64-bit integer multiplication. Here’s an edited version:

Seeing the register keyword always sets alarm bells ringing for me. Not because it has no purpose – you can use it to disallow using the unary address-of operator on a variable, which lets the compiler optimise accesses to that variable – but because it usually indicates that the benchmark has been written with a specific compiler implementation, or version, in mind. LMBench was released in 1996 which would have made GCC 2.7 the current version.

Using the register keyword may have helped old compilers optimise access to variables by allocating registers for them, but modern compilers ignore register when making register allocation decisions.

Before doing anything else, I wanted to verify that the compiler was emitting those 64-bit multiplication operations on lines 17-21 above.

00000000004004cb <do_int64_mul>:
  4004cb:       89 f2                   mov    %esi,%edx
  4004cd:       8d 46 06                lea    0x6(%rsi),%eax
  4004d0:       48 c1 e0 20             shl    $0x20,%rax
  4004d4:       48 8d 84 02 2c 92 00    lea    0x922c(%rdx,%rax,1),%rax
  4004db:       00 
  4004dc:       83 ef 01                sub    $0x1,%edi
  4004df:       83 ff ff                cmp    $0xffffffff,%edi
  4004e2:       75 f8                   jne    4004dc <do_int64_mul+0x11>
  4004e4:       89 c7                   mov    %eax,%edi
  4004e6:       e8 cb ff ff ff          callq  4004b6 <use_int>
  4004eb:       f3 c3                   repz retq 

Nope. There’s a complete lack of 64-bit multiplication anywhere in there. As far as the compiler is concerned, the following C code is equivalent to LMBench’s do_int64_mul():

Which makes the test useless because GCC optimised it away.

Why did GCC optimise out the test?

GCC could tell exactly how many times it needed to add all of those 64-bit constants together and used techniques like Constant folding and propagation to calculate the end value at compile time instead of runtime.

While investigating this issue I discovered that GCC didn’t throw away the useless loop on lines 8-9 because LMBench uses the -O switch which doesn’t include the necessary optimisation flag. Here’s the full list of optimisations and which level they are enabled for.

This is the problem with microbenchmarks that assume a specific toolchain version or implementation – upgrading the toolchain can break them without you realising. Instead of writing the inner loops in C (the authors wanted it to be portable), inline assembly would have prevented the compiler from eliminating them.

Tests like int64-mul are so low-level that I’ve heard them referred to as nanobenchmarks; they are notoriously easy to misuse and misunderstand. Here’s Aleksy Shipilëv, infamous JVM performance expert, showing how to use them with JMH, a benchmark harness:

Is it time to retire LMBench?

As much as I distrust LMBench, I actually plan to keep using it. Why? Because it has some other subtests that are useful, like the fork() microbenchmark test, which detected the overhead of the vm_ops->map_pages() API when it was introduced.

But the CPU ops subtest? No, that nanobenchmark definitely needs to go in the trash.

a silhouette of a person's head and shoulders, used as a default avatar

Manual encryption partition setup for stronger full disk encryption

When installing openSUSE or SUSE Linux Enterprise, YaST is able to configure encrypted LVM using LUKS for full disk encryption. The default configuration is aes-xts-plain64 using a 256 bit master key. However, due to how the XTC mode splits the key into two halves, this reduces the effective key size used for AES to 128 Bits.

In order to use a 512 bit key for 256 effective AES, one needs to perform manual formatting prior to installation:
cryptsetup LuksFormat --key-size 512 /dev/sda1
However the installer suffers from boo#1030299 which prevents it from writing an entry to /etc/crypttab in this instance. This results in a system that is unable to boot after installation.

The work-around is as follows: Boot into the rescue system, open the crypto device and enter the installed system as a chroot:

cryptsetup luksOpen /dev/sda1 crypto
mount /dev/mapper/system-root /mnt
for X in proc dev sys; do mount -bind /$ /mnt/$X; done
chroot /mnt

(This example assumes /dev/sda1 to be the crypto device, and an LVM VG named system with a LV named root, and no separate /boot.)

Then in the chroot, edit /etc/crypttab to have the following line:

crypto /dev/sda1 none none

See man crypttab for additional settings and options. To finalize, regenerate the initrd and reboot

mkinitrd
exit
reboot

A future rewrite of the YaST storage abstraction layer is planned which should address this issue.

the avatar of Klaas Freitag

SMB on openSUSE Conference

The annual openSUSE Conference 2017 is upcoming! osc17finalNext weekend it will be again in the Z-Bau in Nuremberg, Germany.

The conference program is impressive and if you can make it, you should consider stopping by.

Stefan Schäfer from the Invis server project and me will organize a workshop about openSUSE for Small and Medium Business (SMB).

SMB is a long running concern of the heart of the two of us: Both Stefan, who even does it for living, and me have both used openSUSE in the area of SMB for long and we know how well it serves there. Stefan has even initiated the Invis Server Project, which is completely free software and builds on top of the openSUSE distributions. The Invis Server adds a whole bunch of extra functionality to openSUSE that is extremely useful in the special SMB usecase. It came a long way starting as Stefans own project long years ago, evolving as proper maintained openSUSE Spin in OBS with a small, but active community.

The interesting question is how openSUSE, Invis Server and other smaller projects like for example Kraft can unite and offer a reliable maintained and comprehensive solution for this huge group of potential users, that is now locked in to proprietary technologies mainly while FOSS can really make a difference here.

In the workshop we first will introduce the existing projects briefly, maybe discuss some technical questions like integration of new packages in the openSUSE distributions and such, and also touch organizational question like how we want to setup and market openSUSE SMB.

Participants in the workshop should not expect too much presentation. We rather hope for a lively discussion with many people bringing in their projects that might fit, their experiences and ideas. Don’t be shy :-)

a silhouette of a person's head and shoulders, used as a default avatar

ICC Examin 1.0 on Android

ICC Examin allows since version 1.0 ICC Color Profile viewing on the Android mobile platform. ICC Examin shows ICC color profile elements graphically. This way it is much easier to understand the content. Color primaries, white point, curves, tables and color lists are displayed both numerically and as graphics. Matrices, international texts, Metadata are much easier to read.

Features:
* most profile elements from ICC specification version 2 and version 4
* additionally some widely used non standard tag are understood

ICC color profiles are used in photography, print and various operating systems for improving the visual appearance. A ICC profile describes the color response of a color device. Read more about ISO 15076-1:2010 Standard / Specification ICC.1:2010-12 (Profile version 4.3.0.0), color profiles and ICC color management under www.color.org .

The ICC Examin App is completely rewritten in Qt/QML. QML is a declarative language, making it easy to define GUI elements and write layouts with fewer code. In recent years the Qt project extended support from desktop platforms to mobiles like Nokias Meego, Sailfish OS, iOS, Android, embedded devices and more. ICC Examin is available as a paid app in the Google Play Store. Sources are currently closed in order to financially support further development. This ICC Examin version continues to use Oyranos CMS. New is the dependency to RefIccMAX for parsing ICC Profile binaries. In the process both the RefIccMAX library and the Oyranos Color Management System obtained changes and fixes in git for cross compilation with Android libraries. Those changes will be in the next respective releases.

The FLTK Toolkit, as used in previous versions, was not ported to the Android or other mobile platforms. Thus a complete rewrite was unavoidable. The old FLTK based version is still maintained by the same author.