ADD: filesystems: Write-enabled WEBDAV #1567

Merged
merged 1 commit into from Feb 10, 2013

Conversation

Projects
None yet
4 participants
Owner

koying commented Oct 8, 2012

Follow-up from #1164

koying referenced this pull request Oct 8, 2012

Merged

ADD: Write-enabled WEBDAV #1164

@elupus elupus commented on the diff Oct 8, 2012

xbmc/filesystem/CurlFile.cpp
@@ -894,6 +936,82 @@ bool CCurlFile::Open(const CURL& url)
return true;
}
+bool CCurlFile::OpenForWrite(const CURL& url, bool bOverWrite)
+{
+ if(m_opened)
+ return false;
+
+ if (Exists(url) && !bOverWrite)
@elupus

elupus Oct 8, 2012

Member

should be other way around. no need to perform exists check if we set overwrite.

@elupus elupus commented on the diff Oct 8, 2012

xbmc/filesystem/CurlFile.cpp
@@ -1347,6 +1465,13 @@ bool CCurlFile::CReadState::FillBuffer(unsigned int want)
return true;
}
+void CCurlFile::CReadState::SetReadBuffer(const void* lpBuf, int64_t uiBufSize)
+{
+ m_readBuffer = (char*)lpBuf;
+ m_fileSize = uiBufSize;
+ m_filePos = 0;
+}
@elupus

elupus Oct 8, 2012

Member

imho, m_fileSize = m_filePos + uiBufSize only.

@koying

koying Oct 9, 2012

Owner

m_filePos doesn't mean the same thing when reading. When writing, it means "How many bytes were written upstream between 2 pauses".

I should create a more speaking variable, but I find it a bit silly to add 8 bytes just for readability.

@elupus

elupus Oct 9, 2012

Member

My suggested changes aught to have worked the same. Pos show total written,
len giving total written plus what is left.

But we should at some point support open/read/write to get the response
data to the post which might complicate things.

@elupus elupus commented on the diff Oct 8, 2012

xbmc/filesystem/CurlFile.cpp
+
+ if (!m_stillRunning)
+ break;
+
+ if (result != CURLM_OK)
+ {
+ long code;
+ if(g_curlInterface.easy_getinfo(m_state->m_easyHandle, CURLINFO_RESPONSE_CODE, &code) == CURLE_OK )
+ CLog::Log(LOGERROR, "%s - unable to write curl resource (%s) - %ld", __FUNCTION__, m_url.c_str(), code);
+ m_inError = true;
+ return -1;
+ }
+ }
+
+ m_writeOffset += m_state->m_filePos;
+ return m_state->m_filePos;
@elupus

elupus Oct 8, 2012

Member

uiBufSize? everything is always written right?

@elupus

elupus Oct 8, 2012

Member

or uiBufSize + m_state->m_filePos - m_state->m_fileLength, with my other proposed change.

@koying

koying Oct 9, 2012

Owner

Currently, m_filePos (in the "bytes wriiten" sense) is always = uiBufSize.

Conceptually,m_filePos being the number of bytes actually wriiten upstream, it is the one to use.

@elupus elupus commented on the diff Oct 8, 2012

xbmc/filesystem/CurlFile.cpp
@@ -198,6 +227,8 @@ size_t CCurlFile::CReadState::WriteCallback(char *buffer, size_t size, size_t ni
m_cancelled = false;
m_bFirstLoop = true;
m_headerdone = false;
+ m_readBuffer = 0;
+ m_isPaused = false;
@elupus

elupus Oct 8, 2012

Member

do we need this? don't we get proper returns from curl?

@koying

koying Oct 9, 2012

Owner

I didn't find in the CURL API any return code or whatever to indicate that a transfer is paused...

Member

elupus commented Oct 8, 2012

This is looking oh so much better thou. would be nice to get in for frodo. need make sure it doesn't break anything thou.

Member

elupus commented Oct 8, 2012

ps, that leaves tomorrow before merge window closes.

Owner

koying commented Oct 8, 2012

Ok. Will make sure to check your comments in detail tomorrow. I'm CET btw.

Owner

koying commented Oct 9, 2012

Re breaking, I don't think I actually touch anything existing, i.e. CURL reading, so IMO no risks.
Anyway, having ~80% of my files on a webdav share, I'd be the first to notice ;)

Owner

koying commented Oct 25, 2012

rebased

Owner

koying commented Jan 31, 2013

@elupus Still ok with this?

@koying koying added a commit that referenced this pull request Feb 10, 2013

@koying koying Merge pull request #1567 from koying/master
ADD: filesystems: Write-enabled WEBDAV
6f2276c

@koying koying merged commit 6f2276c into xbmc:master Feb 10, 2013

Member

arnova commented Feb 10, 2013

Nice :-)

Owner

koying commented Feb 10, 2013

Hehe.. Better late than never ;)

Owner

Memphiz commented Feb 10, 2013

Next time ping an osx dev before merging stuff which needs xcode project addaptions Grrr

Owner

koying commented Feb 10, 2013

Sorry. Will remember that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment