Skip to content

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Apr 4, 2017

While consultating some code on Storage backends fopen implementation I saw that fseek was attempted on the file string instead of a handle.

After discussing with @icewind1991 the suggestion was to remove and unsupport the slow append mode in general.

Quoting the discussion

@icewind1991 the entire append mode should just be removed, it's barely supported by any storage backend and where it is it's mostly slow
@blizzz then it needs to be removed in favor of read/copy, and overwrite the whole file? just ignoring append wouldn't work would it? or should it simply return false?
@icewind1991 just return falser
@blizzz dropbox and amazon backends just delegate it to native fopen. Swift does essentially the same without this fault fseek, so it should just continue to work while only removing the three lines
@icewind1991 I would return false anyway
apps relying on complex fopen behavior will just hurt future optimizitations
@blizzz and it's not really used anyway? Only possible scenario would be if an app would deliberately make use of this, correct?
@icewind1991 y
@blizzz removing this for nothing would just cut out a standard operation.

So for now I just return false on append operations and removed the broken and unnecessary lines.

@mention-bot
Copy link

@blizzz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @berendt, @icewind1991, @butonic and @Xenopathic to be potential reviewers.

@blizzz blizzz added the 3. to review Waiting for reviews label Apr 4, 2017
@blizzz blizzz force-pushed the append-fixes-swift branch from bcfe1a6 to db07f83 Compare April 4, 2017 12:15
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants