Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.

Fixing some TODOs #20

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/Microsoft.Management.Infrastructure/CimClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,7 @@ internal void AssertNotDisposed()

public override int GetHashCode()
{
// TODO: implement this function?
//return this.ClassHandle.GetClassHashCode();
return 0;
return this.ClassHandle.GetClassHashCode();
}

public override bool Equals(object obj)
Expand Down
46 changes: 46 additions & 0 deletions src/Microsoft.Management.Infrastructure/CimInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,52 @@ private static string PutEscapeCharacterBack(string propValue)
}
*/

internal static object ConvertFromNativeLayer(
MI_Type type,
MI_Value value,
CimInstance parent = null,
bool clone = false)
{
if (type == MI_Type.MI_INSTANCE)
{
CimInstance instance = new CimInstance(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeated code for initialization. Consider helper (e.g. ConvertInstanceFromNativeLayer)

clone ? value.Instance.Clone() : value.Instance);
if (parent != null)
{
instance.SetCimSessionComputerName(parent.GetCimSessionComputerName());
instance.SetCimSessionInstanceId(parent.GetCimSessionInstanceId());
}
return instance;
}

if (type == MI_Type.MI_INSTANCEA)
{
CimInstance[] arrayOfInstances = new CimInstance[value.InstanceA.Length];
for (int i = 0; i < value.InstanceA.Length; i++)
{
MI_Instance h = value.InstanceA[i];
if (h == null)
{
arrayOfInstances[i] = null;
}
else
{
arrayOfInstances[i] = new CimInstance(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CimInstance had better clone the memory, otherwise my memory management pattern of having the underlying native memory be deleted on Dispose won't work

clone ? h.Clone() : h);
if (parent != null)
{
arrayOfInstances[i].SetCimSessionComputerName(parent.GetCimSessionComputerName());
arrayOfInstances[i].SetCimSessionInstanceId(parent.GetCimSessionInstanceId());
}
}
}
return arrayOfInstances;
}

return value;
}


#endregion Helpers

#region IDisposable Members
Expand Down
3 changes: 1 addition & 2 deletions src/Microsoft.Management.Infrastructure/CimSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,7 @@ protected virtual void Dispose(bool disposing)

if (disposing)
{
// TODO: Implement and Call ReleaseHandleAsynchronously
//this._handle.Dispose();
this._handle.Dispose();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static MI_Class Clone(this MI_Class handleToClone)
{
return null;
}
// TODO: handleToClone.AssertValidInternalState();
handleToClone.AssertValidInternalState();

MI_Class clonedHandle;
MI_Result result = handleToClone.Clone(out clonedHandle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ public override CimQualifier this[string qualifierName]

MI_QualifierSet qualifierSet;
MI_Result result = this.classHandle.GetClassQualifierSet(out qualifierSet);
CimException.ThrowIfMiResultFailure(result);
// TODO: there aren't many comments for the above pattern throughout the MMI sources, but if the above fails we shouldn't throw exception, just return MI_RESULT_NOT_FOUND like below. Make sure all of these cases are accounted for in MMI

if (result != MI_Result.MI_RESULT_OK)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a breaking change to the client. Why are you doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I actually need to add a few more conditions than I have it after reviewing the MMIN code for this. Previously, these MI calls (GetClassQualifierSet and GetQualifier below) were done in a single native C++ function called GetMethodQualifierElementAt_GetFlags. If the GetClassQualifierSet function call returned non-MI_RESULT_OK, then it wouldn't try to call GetQualifier, but it also wouldn't necessarily throw an exception either like the previous version of this file had. It would drop right down into the switch block below.

I think a possible solution, if we want to preserve algorithmic equivalency, is to return null if result is MI_RESULT_NOT_FOUND, else if result is not MI_RESULT_OK then call ThrowIfMiResultFailure if it is, and if it is MI_RESULT_OK then just continue on with the function to call GetQualifier.

{
return null;
}

MI_Type qualifierType;
MI_Flags qualifierFlags;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public override string Name
MI_Result result = this.classHandle.GetMethodAt(
(uint)this.index,
out name,
out qualifierSet,
out parameterSet);
out qualifierSet,
out parameterSet);
CimException.ThrowIfMiResultFailure(result);
return name;
}
Expand All @@ -48,8 +48,8 @@ public override CimType ReturnType
MI_Result result = this.classHandle.GetMethodAt(
(uint)this.index,
out name,
out qualifierSet,
out parameterSet);
out qualifierSet,
out parameterSet);
CimException.ThrowIfMiResultFailure(result);
result = parameterSet.GetMethodReturnType(out type, qualifierSet);
CimException.ThrowIfMiResultFailure(result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ public override CimMethodParameterDeclaration this[string parameterName]
out qualifierSet,
out parameterSet);

CimException.ThrowIfMiResultFailure(result);
if (result != MI_Result.MI_RESULT_OK)
{
return null;
}

MI_Type parameterType;
string referenceClass;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ public override CimQualifier this[string qualifierName]
out name,
out qualifierSet,
out parameterSet);
CimException.ThrowIfMiResultFailure(result);

if (result != MI_Result.MI_RESULT_OK)
{
return null;
}

MI_Type parameterType;
string referenceClass;
Expand All @@ -81,7 +85,11 @@ public override CimQualifier this[string qualifierName]
out parameterType,
out referenceClass,
out methodQualifierSet);
CimException.ThrowIfMiResultFailure(result);

if (result != MI_Result.MI_RESULT_OK)
{
return null;
}

MI_Type qualifierType;
MI_Flags qualifierFlags;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ public override CimQualifier this[string methodName]
out name,
out qualifierSet,
out parameterSet);
CimException.ThrowIfMiResultFailure(result);

if (result != MI_Result.MI_RESULT_OK)
{
return null;
}

MI_Type qualifierType;
MI_Flags qualifierFlags;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ public override CimQualifier this[string qualifierName]
out qualifierSet,
out flags,
out index);
CimException.ThrowIfMiResultFailure(result);

if (result != MI_Result.MI_RESULT_OK)
{
return null;
}

MI_Type qualifierType;
MI_Flags qualifierFlags;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ internal void CallIntoUserCallback(

internal abstract void ReportInternalError(CimOperationCallbackProcessingContext callbackProcessingContext, Exception internalError);

private void ReportInternalErrorCore(CimOperationCallbackProcessingContext callbackProcessingContext, Exception internalError)
private void ReportInternalErrorCore(object callbackProcessingContext, Exception internalError)
{
Debug.Assert(internalError != null, "Caller should make sure internalError != null");

Expand All @@ -191,7 +191,7 @@ private void ReportInternalErrorCore(CimOperationCallbackProcessingContext callb
internalErrorWhileCancellingOperation);
}

this.ReportInternalError(callbackProcessingContext, internalError);
this.ReportInternalError((CimOperationCallbackProcessingContext)callbackProcessingContext, internalError);

this._suppressFurtherUserCallbacks = true;
}
Expand All @@ -200,9 +200,8 @@ private void ReportInternalErrorCore(CimOperationCallbackProcessingContext callb

public virtual void RegisterAcceptedAsyncCallbacks(MI_OperationCallbacks operationCallbacks, CimOperationOptions operationOptions)
{
// TODO: Uncomment and fix two lines below
//operationCallbacks.InternalErrorCallback = this.ReportInternalErrorCore;
//operationCallbacks.ManagedOperationContext = this;
operationCallbacks.InternalErrorCallback = this.ReportInternalErrorCore;
operationCallbacks.ManagedOperationContext = this;
}

#endregion Dealing with async callbacks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ internal CimAsyncClassObserverProxy(IObserver<CimClass> observer, bool shortenLi
}

internal void ClassCallback(
CimOperationCallbackProcessingContext callbackProcessingContext,
MI_Operation operationHandle,
object callbackProcessingContext,
MI_Class ClassHandle,
bool moreResults,
MI_Result operationResult,
String errorMessage,
MI_Instance errorDetailsHandle)
MI_Instance errorDetailsHandle,
MI_OperationCallbacks.MI_OperationCallback_ResultAcknowledgement resultAcknowledgement)
{
CimClass currentItem = null;
if ((ClassHandle != null) && (!ClassHandle.IsNull))
Expand All @@ -40,7 +41,7 @@ internal void ClassCallback(

try
{
this.ProcessNativeCallback(callbackProcessingContext, currentItem, moreResults, operationResult, errorMessage, errorDetailsHandle);
this.ProcessNativeCallback((CimOperationCallbackProcessingContext)callbackProcessingContext, currentItem, moreResults, operationResult, errorMessage, errorDetailsHandle);
}
finally
{
Expand All @@ -57,8 +58,7 @@ internal void ClassCallback(
public override void RegisterAcceptedAsyncCallbacks(MI_OperationCallbacks operationCallbacks, CimOperationOptions operationOptions)
{
base.RegisterAcceptedAsyncCallbacks(operationCallbacks, operationOptions);
// TODO: Uncomment and fix below
//operationCallbacks.classResult = this.ClassCallback;
operationCallbacks.classResult = this.ClassCallback;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@ internal CimAsyncIndicationObserverProxy(IObserver<CimSubscriptionResult> observ
}

internal void IndicationResultCallback(
CimOperationCallbackProcessingContext callbackProcessingContext,
MI_Operation operationHandle,
object callbackProcessingContext,
MI_Instance instanceHandle,
String bookMark,
String machineID,
bool moreResults,
MI_Result operationResult,
String errorMessage,
MI_Instance errorDetailsHandle)
MI_Instance errorDetailsHandle,
MI_OperationCallbacks.MI_OperationCallback_ResultAcknowledgement resultAcknowledgement)
{
CimSubscriptionResult currentItem = null;
if ((instanceHandle != null) && (!instanceHandle.IsNull))
Expand All @@ -42,7 +43,7 @@ internal void IndicationResultCallback(

try
{
this.ProcessNativeCallback(callbackProcessingContext, currentItem, moreResults, operationResult, errorMessage, errorDetailsHandle);
this.ProcessNativeCallback((CimOperationCallbackProcessingContext)callbackProcessingContext, currentItem, moreResults, operationResult, errorMessage, errorDetailsHandle);
}
finally
{
Expand All @@ -59,8 +60,7 @@ internal void IndicationResultCallback(
public override void RegisterAcceptedAsyncCallbacks(MI_OperationCallbacks operationCallbacks, CimOperationOptions operationOptions)
{
base.RegisterAcceptedAsyncCallbacks(operationCallbacks, operationOptions);
// TODO: Uncomment and fix below
//operationCallbacks.indicationResult = this.IndicationResultCallback;
operationCallbacks.indicationResult = this.IndicationResultCallback;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ internal CimAsyncInstanceObserverProxy(IObserver<CimInstance> observer,
}

internal void InstanceResultCallback(
CimOperationCallbackProcessingContext callbackProcessingContext,
MI_Operation operationHandle,
object callbackProcessingContext,
MI_Instance instanceHandle,
bool moreResults,
MI_Result operationResult,
String errorMessage,
MI_Instance errorDetailsHandle)
MI_Instance errorDetailsHandle,
MI_OperationCallbacks.MI_OperationCallback_ResultAcknowledgement resultAcknowledgement)
{
CimInstance currentItem = null;
if ((instanceHandle != null) && (!instanceHandle.IsNull))
Expand All @@ -49,7 +50,7 @@ internal void InstanceResultCallback(

try
{
this.ProcessNativeCallback(callbackProcessingContext, currentItem, moreResults, operationResult, errorMessage, errorDetailsHandle);
this.ProcessNativeCallback((CimOperationCallbackProcessingContext)callbackProcessingContext, currentItem, moreResults, operationResult, errorMessage, errorDetailsHandle);
}
finally
{
Expand All @@ -66,8 +67,7 @@ internal void InstanceResultCallback(
public override void RegisterAcceptedAsyncCallbacks(MI_OperationCallbacks operationCallbacks, CimOperationOptions operationOptions)
{
base.RegisterAcceptedAsyncCallbacks(operationCallbacks, operationOptions);
// TODO: Uncomment and fix below
//operationCallbacks.instanceResult = this.InstanceResultCallback;
operationCallbacks.instanceResult = this.InstanceResultCallback;
}
}
}
Loading