OLEAUT32: implementation for IPersistPropertyBag_Load for OLEFont
Robert Shearman
rob at codeweavers.com
Wed Aug 18 14:01:14 CDT 2004
Alex Villacís Lasso wrote:
> This implementation is enough to satisfy VB6 when loading font objects
> from property bags in projects.
>
> Changelog:
> * Added implementation for IPersistPropertyBag_Load on OLEFont
Looks good, but a few comments below:
...
>+ V_VT(&valueAttr) = VT_EMPTY;
>
>
It is probably better practice to use VariantInit here.
>+ iRes = IPropertyBag_Read(pPropBag, sAttrName, &valueAttr, pErrorLog);
>+ switch (iRes) {
>
>
>+ case S_OK:
>+ if (V_VT(&valueAttr) == VT_BSTR) {
>+ /* Type is BSTR, use for name directly */
>+ OLEFontImpl_put_Name(this, V_BSTR(&valueAttr));
>+
>+ /* FIXME: Leaking memory here? */
>+ } else {
>+ /* Don't know how to handle this type */
>+ FIXME("Name has type %d, not implemented\n", V_VT(&valueAttr));
>+ iRes = E_NOTIMPL;
>+ }
>
Perhaps use VariantChangeType here?
>+ break;
>+ case E_POINTER:
>+ FIXME("\tiResult = [E_POINTER]\n");
>+ break;
>+ case E_INVALIDARG:
>+ FIXME("\tiResult = [undef]\n");
>+ iRes = S_OK;
>+ break;
>+ case E_FAIL:
>+ FIXME("\tiResult = [E_FAIL]\n");
>+ break;
>+ }
>
Catch the errors you need to (why is E_INVALIDARG ok?) and either move
these common debug statements to the end of the function (they should
also be ERR's, not FIXME's). Also, you other unexpected errors, which is
bad.
>+ }
>+
>+ if (iRes == S_OK) {
>+ CY iSize;
>+ iSize.s.Hi = 0;
>+ iSize.s.Lo = 0;
>+
>+ V_VT(&valueAttr) = VT_EMPTY;
>+ iRes = IPropertyBag_Read(pPropBag, sAttrSize, &valueAttr, pErrorLog);
>+ switch (iRes) {
>+ case S_OK:
>+ switch (V_VT(&valueAttr)) {
>+ case VT_I2:
>+ iSize.s.Lo = V_I2(&valueAttr) * 10000UL;
>+ break;
>+ case VT_UI2:
>+ iSize.s.Lo = V_UI2(&valueAttr) * 10000UL;
>+ break;
>+ case VT_I4:
>+ iSize.s.Lo = V_I4(&valueAttr) * 10000UL;
>+ break;
>+ case VT_UI4:
>+ iSize.s.Lo = V_UI4(&valueAttr) * 10000UL;
>+ break;
>+ case VT_R4:
>+ iSize.s.Lo = (ULONG)(V_R4(&valueAttr) * 10000UL);
>+ break;
>+ case VT_R8:
>+ iSize.s.Lo = (ULONG)(V_R8(&valueAttr) * 10000UL);
>+ break;
>
>
Again, perhaps use VariantChangeType here?
>+ default:
>+ /* Don't know how to handle this type */
>+ FIXME("Size type %d, not implemented\n", V_VT(&valueAttr));
>+ iRes = E_NOTIMPL;
>+ break;
>+ }
>+ if (iRes != E_NOTIMPL) IFont_put_Size(this, iSize);
>+ break;
>+ case E_POINTER:
>+ FIXME("\tiResult = [E_POINTER]\n");
>+ break;
>+ case E_INVALIDARG:
>+ FIXME("\tiResult = [undef]\n");
>+ iRes = S_OK;
>+ break;
>+ case E_FAIL:
>+ FIXME("\tiResult = [E_FAIL]\n");
>+ break;
>+ }
>
Same comment as before about errors.
>+ }
>+ if (iRes == S_OK) {
>+ signed short iCharset = 0;
>+
>+ V_VT(&valueAttr) = VT_EMPTY;
>+ iRes = IPropertyBag_Read(pPropBag, sAttrCharset, &valueAttr, pErrorLog);
>+ switch (iRes) {
>+ case S_OK:
>+ switch (V_VT(&valueAttr)) {
>+ case VT_I2:
>+ if (V_I2(&valueAttr) < 0) {
>+ FIXME("Unexpected value for Font Charset (%hd), using 0\n", V_I2(&valueAttr));
>+ } else {
>+ iCharset = V_I2(&valueAttr);
>+ }
>+ break;
>+ case VT_UI2:
>+ if (V_UI4(&valueAttr) > 32767L) {
>+ FIXME("Unexpected value for Font Charset (%ld), using 0\n", V_UI4(&valueAttr));
>+ } else {
>+ iCharset = V_UI2(&valueAttr);
>+ }
>+ break;
>+ case VT_I4:
>+ if (V_I4(&valueAttr) < 0 || V_I4(&valueAttr) > 32767L) {
>+ FIXME("Unexpected value for Font Charset (%ld), using 0\n", V_I4(&valueAttr));
>+ } else {
>+ iCharset = V_I4(&valueAttr);
>+ }
>+ break;
>+ case VT_UI4:
>+ if (V_UI4(&valueAttr) > 32767L) {
>+ FIXME("Unexpected value for Font Charset (%ld), using 0\n", V_UI4(&valueAttr));
>+ } else {
>+ iCharset = V_UI4(&valueAttr);
>+ }
>+ break;
>+ default:
>+ /* Don't know how to handle this type */
>+ FIXME("Charset type %d, not implemented\n", V_VT(&valueAttr));
>+ iRes = E_NOTIMPL;
>
>
Use VariantChangeType here and you will get all the bounds checking for
free.
>+ }
>+ if (iRes != E_NOTIMPL) IFont_put_Charset(this, iCharset);
>+ break;
>+ case E_POINTER:
>+ FIXME("\tiResult = [E_POINTER]\n");
>+ break;
>+ case E_INVALIDARG:
>+ FIXME("\tiResult = [undef]\n");
>+ iRes = S_OK;
>+ break;
>+ case E_FAIL:
>+ FIXME("\tiResult = [E_FAIL]\n");
>+ break;
>+ }
>+ }
>+
>+ if (iRes == S_OK) {
>+ signed short iWeight = FW_NORMAL;
>+
>+ V_VT(&valueAttr) = VT_EMPTY;
>+ iRes = IPropertyBag_Read(pPropBag, sAttrWeight, &valueAttr, pErrorLog);
>+ switch (iRes) {
>+ case S_OK:
>+ switch (V_VT(&valueAttr)) {
>+ case VT_I2:
>+ if (V_I2(&valueAttr) < 0) {
>+ FIXME("Unexpected value for Font Weight (%hd), using FW_NORMAL\n", V_I2(&valueAttr));
>+ } else {
>+ iWeight = V_I2(&valueAttr);
>+ }
>+ break;
>+ case VT_UI2:
>+ if (V_UI4(&valueAttr) > 32767L) {
>+ FIXME("Unexpected value for Font Weight (%ld), using FW_NORMAL\n", V_UI4(&valueAttr));
>+ } else {
>+ iWeight = V_UI2(&valueAttr);
>+ }
>+ break;
>+ case VT_I4:
>+ if (V_I4(&valueAttr) < 0 || V_I4(&valueAttr) > 32767L) {
>+ FIXME("Unexpected value for Font Weight (%ld), using FW_NORMAL\n", V_I4(&valueAttr));
>+ } else {
>+ iWeight = V_I4(&valueAttr);
>+ }
>+ break;
>+ case VT_UI4:
>+ if (V_UI4(&valueAttr) > 32767L) {
>+ FIXME("Unexpected value for Font Weight (%ld), using FW_NORMAL\n", V_UI4(&valueAttr));
>+ } else {
>+ iWeight = V_UI4(&valueAttr);
>+ }
>+ break;
>+ default:
>+ /* Don't know how to handle this type */
>+ FIXME("Weight type %d, not implemented\n", V_VT(&valueAttr));
>+ iRes = E_NOTIMPL;
>+ }
>
Again, use VariantChangeType.
>+ if (iRes != E_NOTIMPL) IFont_put_Weight(this, iWeight);
>+ break;
>+ case E_POINTER:
>+ FIXME("\tiResult = [E_POINTER]\n");
>+ break;
>+ case E_INVALIDARG:
>+ FIXME("\tiResult = [undef]\n");
>+ iRes = S_OK;
>+ break;
>+ case E_FAIL:
>+ FIXME("\tiResult = [E_FAIL]\n");
>+ break;
>+ }
>+ }
>+
>+ if (iRes == S_OK) {
>+ BOOL bUnderline = FALSE;
>+
>+ V_VT(&valueAttr) = VT_EMPTY;
>+ iRes = IPropertyBag_Read(pPropBag, sAttrUnderline, &valueAttr, pErrorLog);
>+ switch (iRes) {
>+ case S_OK:
>+ switch (V_VT(&valueAttr)) {
>+ case VT_I2:
>+ bUnderline = V_I2(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_I4:
>+ bUnderline = V_I4(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_UI2:
>+ bUnderline = V_UI2(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_UI4:
>+ bUnderline = V_UI4(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_BOOL:
>+ bUnderline = V_BOOL(&valueAttr);
>+ break;
>+ default:
>+ /* Don't know how to handle this type */
>+ FIXME("Underline type %d, not implemented\n", V_VT(&valueAttr));
>+ iRes = E_NOTIMPL;
>+ break;
>+ }
>
Again, VariantChangeType.
>+ if (iRes != E_NOTIMPL) IFont_put_Underline(this, bUnderline);
>+ break;
>+ case E_POINTER:
>+ FIXME("\tiResult = [E_POINTER]\n");
>+ break;
>+ case E_INVALIDARG:
>+ FIXME("\tiResult = [undef]\n");
>+ iRes = S_OK;
>+ break;
>+ case E_FAIL:
>+ FIXME("\tiResult = [E_FAIL]\n");
>+ break;
>+ }
>+ }
>+
>+ if (iRes == S_OK) {
>+ BOOL bItalic = FALSE;
>+
>+ V_VT(&valueAttr) = VT_EMPTY;
>+ iRes = IPropertyBag_Read(pPropBag, sAttrItalic, &valueAttr, pErrorLog);
>+ switch (iRes) {
>+ case S_OK:
>+ switch (V_VT(&valueAttr)) {
>+ case VT_I2:
>+ bItalic = V_I2(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_I4:
>+ bItalic = V_I4(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_UI2:
>+ bItalic = V_UI2(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_UI4:
>+ bItalic = V_UI4(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_BOOL:
>+ bItalic = V_BOOL(&valueAttr);
>+ break;
>+ default:
>+ /* Don't know how to handle this type */
>+ FIXME("Italic type %d, not implemented\n", V_VT(&valueAttr));
>+ iRes = E_NOTIMPL;
>+ break;
>+ }
>
Again, use VariantChangeType.
>+ if (iRes != E_NOTIMPL) IFont_put_Italic(this, bItalic);
>+ break;
>+ case E_POINTER:
>+ FIXME("\tiResult = [E_POINTER]\n");
>+ break;
>+ case E_INVALIDARG:
>+ FIXME("\tiResult = [undef]\n");
>+ iRes = S_OK;
>+ break;
>+ case E_FAIL:
>+ FIXME("\tiResult = [E_FAIL]\n");
>+ break;
>+ }
>+ }
>+
>+ if (iRes == S_OK) {
>+ BOOL bStrikethrough = FALSE;
>+
>+ V_VT(&valueAttr) = VT_EMPTY;
>+ iRes = IPropertyBag_Read(pPropBag, sAttrStrikethrough, &valueAttr, pErrorLog);
>+ switch (iRes) {
>+ case S_OK:
>+ switch (V_VT(&valueAttr)) {
>+ case VT_I2:
>+ bStrikethrough = V_I2(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_I4:
>+ bStrikethrough = V_I4(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_UI2:
>+ bStrikethrough = V_UI2(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_UI4:
>+ bStrikethrough = V_UI4(&valueAttr) ? TRUE : FALSE;
>+ break;
>+ case VT_BOOL:
>+ bStrikethrough = V_BOOL(&valueAttr);
>+ break;
>+ default:
>+ /* Don't know how to handle this type */
>+ FIXME("Strikethrough type %d, not implemented\n", V_VT(&valueAttr));
>+ iRes = E_NOTIMPL;
>+ break;
>+ }
>
>
Again, use VariantChangeType.
>+ if (iRes != E_NOTIMPL) IFont_put_Strikethrough(this, bStrikethrough);
>+ break;
>+ case E_POINTER:
>+ FIXME("\tiResult = [E_POINTER]\n");
>+ break;
>+ case E_INVALIDARG:
>+ FIXME("\tiResult = [undef]\n");
>+ iRes = S_OK;
>+ break;
>+ case E_FAIL:
>+ FIXME("\tiResult = [E_FAIL]\n");
>+ break;
>+ }
>+ }
>+ return iRes;
> }
>
> static HRESULT WINAPI OLEFontImpl_IPersistPropertyBag_Save(
>
>
Thanks,
Rob
More information about the wine-devel
mailing list